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

select.poll not available on OS X #4

Open
jrs65 opened this issue Jul 27, 2016 · 6 comments
Open

select.poll not available on OS X #4

jrs65 opened this issue Jul 27, 2016 · 6 comments

Comments

@jrs65
Copy link

jrs65 commented Jul 27, 2016

I've been trying to run the sky field tests, and having issues.

Though from the comments in the code, this failure seems to be expected, I just want to confirm that in my version of Python (2.7.10) on OS X (10.11.6), select.poll is not available at all, and so assay crashes:

$ assay --batch skyfield.tests
Traceback (most recent call last):
  File "/Users/richard/Projects/pipeline_refactor/skyfieldtest/bin/assay", line 9, in <module>
    load_entry_point('assay==0.0', 'console_scripts', 'assay')()
  File "/Users/richard/Projects/pipeline_refactor/assay/assay/command.py", line 22, in main
    monitor.main_loop(args.name, args.batch or not isatty)
  File "/Users/richard/Projects/pipeline_refactor/assay/assay/monitor.py", line 39, in main_loop
    poller = unix.EPoll()
  File "/Users/richard/Projects/pipeline_refactor/assay/assay/unix.py", line 77, in __init__
    self.poller = select.poll()  # TODO: does this work on OS X?
AttributeError: 'module' object has no attribute 'poll'

It seems that possible options to get this one working could be moving away from poll to select which should be supported on OS X as well as Linux (though has a few limitations), or adding in support for Kqueues

@brandon-rhodes brandon-rhodes self-assigned this Jul 29, 2016
@brandon-rhodes
Copy link
Owner

I'd welcome a pull request making this improvement, but it's time to admit to myself that I'm not getting around to it "this weekend" — so I'm going to unassign myself so folks don't think I'm actively working on this!

@brandon-rhodes brandon-rhodes removed their assignment Jun 30, 2018
@dusty-phillips
Copy link

I'm playing around with this today. Not expecting to get anywhere, but I noticed:

  • Someone's forked assay and made a KQueue compatible queue here: https://github.com/dkua/assay/tree/kq I haven't tested it yet, but the code "looks right".
  • The Filesystem class that depends on inotify will not work on macos. There is a trivial error trying to import libc.so.6 (needs to be "libc.dylib" if sys.platform == "darwin" else "libc.so.6") but after fixing that, I get errors on inotify_add_watch: symbol not found. As far as I can tell, inotify doesn't work on the BSDs. My inclination would be to depend on something like watchdog for cross-platform file watching, but I notice you don't currently have any third-party deps in assay. Is that a requirement?

@brandon-rhodes
Copy link
Owner

Thanks for updating this issue with a bit more information! I'll think about whether in the future I might want watchdog as a dependency; when I first wrote assay I wanted to go ultra-minimalist because of how fatigued I was from py.test's ultra-maximalism, which is why you see no dependencies.

I wonder if we could make a simple adjustment, so that if Assay detects that it's not on Linux, it falls back to "--batch" mode? Then folks could still get test results instead of a traceback. Then we (I think?) would only need a "poll" alternative, but not watchdog, which would defer the dependency question for a while.

@dusty-phillips
Copy link

I’ll test that out tomorrow and see if the kqueue implementation works.

I dug through the watchdog sources today and decided it isn’t the right tool for assay anyway. It provides an abstracted interface across platforms which means you can’t listen on the same epoll/kqueue interface for the worker processes and file system events. It seems silly to add an extra layer of abstraction to unabstract that which was already abstracted. (I feel like that line is going to make it into one of your future talks…)

I took a look to see if I could extract the macOS polling piece. It wouldn’t be too hard; they ship a small c extension that wraps fsevent, which, as far as I can tell, is the bsd version of inotify. It’s probably possible to borrow or adapt that file (assuming licenses align) and vendor it in. Would result in some cross-platform awkwardness to compile the extension in the setup.py, though. Possibly there is a lighter-than-watchdog fsevent implementation that can be pulled in as an optional dependency for macOS users. Or maybe the magic of ctypes would be sufficient.

@dusty-phillips
Copy link

Running with the KQueue implementation I linked earlier and passing --batch is just hanging. I'll try to dig further today.

@dusty-phillips
Copy link

Attached PR may need some work, but it solves this issue. I'm going to split the filesystem bits out to another issue.

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

No branches or pull requests

3 participants