-
Notifications
You must be signed in to change notification settings - Fork 34
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
Allow using dill for pickling #36
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
# flake8: noqa | ||
try: | ||
from multiprocess import * | ||
from multiprocess import connection, managers, util | ||
except ImportError: | ||
from multiprocessing import * | ||
from multiprocessing import connection, managers, util |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,6 @@ | ||
import pickle | ||
import unittest | ||
|
||
from aioprocessing.executor import _ExecutorMixin | ||
|
||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,8 +1,8 @@ | ||
import unittest | ||
import aioprocessing | ||
from multiprocessing import Process, Event | ||
from concurrent.futures import ProcessPoolExecutor | ||
|
||
import aioprocessing | ||
from aioprocessing.mp import Process, Event, util | ||
from ._base_test import BaseTest, _GenMixin | ||
|
||
|
||
|
@@ -97,6 +97,11 @@ async def queue_put(): | |
|
||
|
||
class ManagerQueueTest(BaseTest): | ||
@unittest.skipIf( | ||
"multiprocess.util" in str(util), | ||
"concurrent.futures is not yet supported by uqfoundation " | ||
"(https://github.com/uqfoundation/pathos/issues/90)" | ||
) | ||
Comment on lines
+100
to
+104
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. sub-optimal to skip this test... especially for people that are using Here's the traceback for this test case, caused by ProcessPoolExecutor internally using test_executor (tests.queue_test.ManagerQueueTest) ... Process SpawnProcess-1:
Traceback (most recent call last):
File "~/.pyenv/versions/3.8.6/lib/python3.8/multiprocessing/process.py", line 315, in _bootstrap
self.run()
File "~/.pyenv/versions/3.8.6/lib/python3.8/multiprocessing/process.py", line 108, in run
self._target(*self._args, **self._kwargs)
File "~/.pyenv/versions/3.8.6/lib/python3.8/concurrent/futures/process.py", line 233, in _process_worker
call_item = call_queue.get(block=True)
File "~/.pyenv/versions/3.8.6/lib/python3.8/multiprocessing/queues.py", line 116, in get
return _ForkingPickler.loads(res)
File "~/.pyenv/versions/pi/lib/python3.8/site-packages/multiprocess/managers.py", line 959, in RebuildProxy
return func(token, serializer, incref=incref, **kwds)
File "~/.pyenv/versions/pi/lib/python3.8/site-packages/multiprocess/managers.py", line 809, in __init__
self._incref()
File "~/.pyenv/versions/pi/lib/python3.8/site-packages/multiprocess/managers.py", line 863, in _incref
conn = self._Client(self._token.address, authkey=self._authkey)
File "~/.pyenv/versions/pi/lib/python3.8/site-packages/multiprocess/connection.py", line 511, in Client
answer_challenge(c, authkey)
File "~/.pyenv/versions/pi/lib/python3.8/site-packages/multiprocess/connection.py", line 762, in answer_challenge
raise AuthenticationError('digest sent was rejected')
multiprocess.context.AuthenticationError: digest sent was rejected There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. note that even if uqfoundation would add compatibility, and expose There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think there should be a note in the readme about this incompatibility when using dill, as well as documenting the workaround that forces usage of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sure thing, added. Assuming semver and this being a potentially breaking change ^ I went ahead and bumped to 2.0.0 🤙 |
||
def test_executor(self): | ||
m = aioprocessing.AioManager() | ||
q = m.AioQueue() | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These imports should still come from
threading
. They're getting proxied, so we don't need the multiprocessing versions.