New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Auto-linewise is still broken? #767

Closed
bitprophet opened this Issue Oct 28, 2012 · 3 comments

Comments

Projects
None yet
2 participants
@bitprophet
Member

bitprophet commented Oct 28, 2012

See most comments after closure of #482 -- anecdotal evidence suggests this feature is broken, even though I can't actually replicate the behavior.

@bitprophet

This comment has been minimized.

Member

bitprophet commented Oct 28, 2012

I still can't replicate the symptom, but I can confirm that the tasks being run don't see env.linewise = True, so...yea.

Guessing it's because the code setting this is a context manager wrapping the definition of the function passed into multiprocessing -- not surprised that relying on global module state could cause hijinx like this.

What is odd is that the env.parallel = True is sticking. If the inner scope was completely ignoring the env changes I'd expect that to also be False.

Will experiment with explicitly handing a copy of local_env down the call chain, and hope that moving things around won't sink some other edge case not under study here 😕

@bitprophet

This comment has been minimized.

Member

bitprophet commented Oct 29, 2012

  • Added an explicit, currently-failing test for this (re: the env var itself)
  • Implemented the above experiment re: being more direct with application of the inner fab env
  • Entire test suite now passes, including new test
  • Manual test that tried to replicate the bug still works fine for me (re: a moderately sized parallel flight running a nontrivial multiline-output command)

I'm committing this change to the 1.4 branch and merging it into master on the assumption that it does fix the problem. Attn @shaftoe & @dmcquay if either of you are up to testing from a Git checkout :)

bitprophet added a commit that referenced this issue Oct 29, 2012

@dmcquay

This comment has been minimized.

dmcquay commented Oct 29, 2012

Thanks @bitprophet! I'll give this a try soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment