[pony-build] Context Failures

[C. Titus Brown]

On Sat, Apr 10, 2010 at 05:29:38PM -0400, Max Laite wrote:
> Looking for some suggestions on how to go about doing this context failure
> stuff. Which we can assume should work for all context classes if we want it
> to and not just Virtualenv.

[ ... ]

Hey Max,

I merged my latest into your 'env' branch, resolved a conflict, and pushed
it to my 'max_env' branch.  You should be able to pull from that branch
into your 'env' branch w/o any conflicts.

A couple of comments --

---

The code in this branch contains a syntax error because of a bug in your
log_level change:

   http://github.com/mlaite/pony-build/commit/a2634e60c3c89de987bed9580552f01fa71be058

So not only are you not running your own code, you're not running the
tests.  Tsk, tsk...

Also, I think the log level is redundant with my --verbose and --debug
changes.  What do you think?  That redundancy isn't necessarily bad;
it might be nice to set a log level explicitly.  If you agree, could
you call set_log_level, if-and-only-if -l is specified, just before
'parse_cmdline' returns?

Finally, please do this work in a branch separate from your context stuff.
You can do this pretty easily --

  git fetch ctb master:ctb   # fetch my latest into local branch 'ctb'
  git checkout ctb

  git cherry-pick a2634e60c3   # grab your log_level patch.

  ... continue work as usual ...

This will let me merge it in independently of your Context work.

--

OK, on to the context stuff!

globals are bad, for testing if nothing else -- imagine trying to test
this code, you'd have to reset the error_state variable every time.  Can
you think of an object or scope to which to attach this state variable?

What happens in your code if context is None?

Also, error_state should be set if 'c.run(...)' ~ line 813 raises an
exception, and the for loop should abort at that point (although cleanup
code should still run).

In case c.run raises an exception, how should that exception be
communicated to the server, do you think?

That 'if error_state' code is really ugly; you're duplicating the code at the
end of the function.  Is there a way to refactor the code so that the 'exit'
cleanup code (file upload, etc.) is all at the end of the 'do' function?

--

cheers,
--titus