Skip to content
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

neat.distributed: use socket module instead of multiprocessing.managers #125

Open
wants to merge 25 commits into
base: master
Choose a base branch
from

Conversation

bennr01
Copy link
Contributor

@bennr01 bennr01 commented Feb 5, 2018

I finally found time to complete the rewrite of distributed.py.
This PR replaces the usage of the multiprocessing.managers module with the socket module.
Here are some advantages of this change:

  • improved stability and performance (tests will no longer fail due to timeout errors)
  • we can now use our own serialization functions instead of relying on pickle, which may be useful if we implement xml/json/... serialization)

These changes are mostly compatible with the old API, however the type of the authkey argument has changed from bytes to str/unicode.

Also, this closes #101.

Edit: It seems like one of the tests still fails randomly. The travis build prior to the PR still succeeded.
Edit 2: Seems like the travis failure was caused by some process timeout. After restarting the build, the tests pass, but coverage is still reduced. However, most of the missing coverage seems to be from error catching code, which cant easily be triggered manually (e.g. handling of invalid network messages).

Benjamin Vogt and others added 22 commits November 23, 2017 11:54
resolved Conflicts:
	neat/distributed.py
remove the travis_wait command.
IIRC, this was once added to fix a few bugs with
distributed+multiprocessing on travis.
Since i want to completely fix distributed, these commands should no
longer be required.
This should improve the runtime of the tests and allow realtime output
of travis.
@coveralls
Copy link

coveralls commented Feb 5, 2018

Coverage Status

Coverage decreased (-1.0%) to 95.171% when pulling 2ed502b on bennr01:distributed_socket into c2b79c8 on CodeReclaimers:master.

@CodeReclaimers
Copy link
Owner

Apologies for the long silence--I've been overwhelmed with other stuff and just kind of let this fall idle. I appreciate the patch and will take a look as soon as I get some free time. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

distributed.py - definitely needs update from stopevent; having headaches coding
3 participants