disable multiprocessing if unavailable #39
Conversation
| @@ -611,7 +628,11 @@ def s3_iter_bucket(bucket, prefix='', accept_key=lambda key: True, key_limit=Non | |||
| if key_limit is not None and key_no + 1 >= key_limit: | |||
| # we were asked to output only a limited number of keys => we're done | |||
| break | |||
| pool.terminate() | |||
|
|
|||
| try: | |||
There was a problem hiding this comment.
Cleaner to check against NO_MULTIPROCESSING, same way as above, rather than try-catch.
There was a problem hiding this comment.
@jsphpl Can you fix this? Then we're ready to merge (the tests we can add later). Cheers!
There was a problem hiding this comment.
PR updated. But still the 2.6 tests fail due to a timeout when testing s3_iter_bucket() - any idea about that?
|
Thanks @jsphpl . The py2.6 tests pass too, it was just some Travis glitch. Can you add a test around this |
|
@piskvorky i could implement it, but have no clue how to approach this. Any ideas?
Would be great if you could point me in the right direction, as i have zero experience with mock & moto… Thanks! |
|
Sure, no problem. I was thinking something simple:
|
|
Sounds good. But how, again, would you simulate the import error for multiprocessing, in order to check if the flag is set? I couldn't find anything about that in pyunit related docs, nor can i think of a creative solution to this… |
|
I didn't even think of testing that automatically. Just test that it works for you / solves your use case. In the unit tests, I thought we'd set the flag manually (force both |
|
Great, let’s do it like that… Joseph Paul Am 12. November 2015 bei 13:40:52, Radim Řehůřek (notifications@github.com) schrieb: I didn't even think of testing that. I just thought we'd set the flag manually (True/False) and see that both code paths work. — |
… the worker pool in s3_iter_bucket
disable multiprocessing if unavailable
|
Thanks for the |
|
@jsphpl Could we set the flag and test in the unit tests? (both True and False) |
|
@tmylk i'm having trouble with the test environment on my machine and don't want to push untested tests ;). So if you feel like writing the tests, please feel free! I'd be thankful if you could help! Otherwise i'll have to get the tests running… |
|
@jsphpl Feel free to push "untested tests". They only take 30 sec to run and Travis is free. :) |
|
@tmylk True, i'll give it a shot. |
| except ImportError: | ||
| logger.warning("multiprocessing could not be imported and won't be used") | ||
| NO_MULTIPROCESSING = True | ||
| from itertools import imap |
There was a problem hiding this comment.
I think this might break Python 3. imap is only available in Python 2.
There was a problem hiding this comment.
@tmylk how come the unit tests have been passing for a year then?
I hacked something up, please check if you're okay with it. Couldn't get the tests running, so this code was tested only manually – not sure if these code changes require the tests to be updated.