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

Don't import luigi.worker from __init__.py #438

Merged
merged 1 commit into from
Sep 9, 2014

Conversation

erikbern
Copy link
Contributor

@erikbern erikbern commented Sep 9, 2014

There's some really fishy bug going on with multiprocessing that we are investigating. It seems like (but still to be confirmed) that luigi.hadoop ends up importing luigi.worker which in turn imports multiprocessing. multiprocessing seems to have a bug causing an exception in the global scope in case the temporary directory is too long (bind() fails if the temp path is > 107 characters, this is a known UNIX issue)

Even if the theory above isn't correct, this still cleans some import dependencies, by moving Event to its own module

@freider
Copy link
Contributor

freider commented Sep 9, 2014

lgtm!

There's some really fishy bug going on with multiprocessing that we are investigating. It *seems* like (but still to be confirmed) that luigi.hadoop ends up importing luigi.worker which in turn imports multiprocessing. multiprocessing *seems* to have a bug causing an exception in the global scope in case the temporary directory is too long (bind() fails if the temp path is > 107 characters, this is a known UNIX issue)

Exception from a Hadoop job:

  __import__('pkg_resources').declare_namespace(__name__)
Process SyncManager-1:
Traceback (most recent call last):
  File "/usr/lib/python2.6/multiprocessing/process.py", line 232, in
_bootstrap
    self.run()
  File "/usr/lib/python2.6/multiprocessing/process.py", line 88, in run
    self._target(*self._args, **self._kwargs)
  File "/usr/lib/python2.6/multiprocessing/managers.py", line 517, in
_run_server
    server = cls._Server(registry, address, authkey, serializer)
  File "/usr/lib/python2.6/multiprocessing/managers.py", line 136, in
__init__
    self.listener = Listener(address=address, backlog=5)
  File "/usr/lib/python2.6/multiprocessing/connection.py", line 106, in
__init__
    self._listener = SocketListener(address, family, backlog)
  File "/usr/lib/python2.6/multiprocessing/connection.py", line 227, in
__init__
    self._socket.bind(address)
  File "<string>", line 1, in bind
error: AF_UNIX path too long

Even if the theory above isn't correct, this still cleans up some import dependencies, by moving Event to its own module
@Tarrasch
Copy link
Contributor

Tarrasch commented Sep 9, 2014

Thanks for the proper commit message! :)

LGTM

@erikbern
Copy link
Contributor Author

erikbern commented Sep 9, 2014

I've always suspected there's an inverse correlation between commit size and commit message length :)

erikbern pushed a commit that referenced this pull request Sep 9, 2014
Don't import luigi.worker from __init__.py
@erikbern erikbern merged commit 2a32a60 into master Sep 9, 2014
@Tarrasch Tarrasch deleted the erikbern/import-worker-event branch September 9, 2014 15:57
Tarrasch added a commit that referenced this pull request Aug 1, 2016
It was introduced in #438. Would that bug be reintroduced I'm think that (1) we have tests nowadays for it and (2) this comment doesn't help any more.
Tarrasch added a commit that referenced this pull request Aug 24, 2016
It was introduced in #438. Would that bug be reintroduced I'm think that (1) we have tests nowadays for it and (2) this comment doesn't help any more.
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.

3 participants