Skip to content

Conversation

jensbjorgensen
Copy link
Contributor

It's quite natural for filesystem monitoring to be part of the event loop, and libuv has done a cross-platform implementation. Actually asyncio should probably expose this as well--maybe they will draw from this example implementation to inform their own api?

I wasn't sure what the best way to expose the FS_EVENT_* constants was--I preferred to have them refer directly to the constants defined in libuv but I'm a cython newb and I couldn't get the right incantation. In the merge below they are hardcoded in uvloop.const. Maybe you can suggest a better way.

@jensbjorgensen
Copy link
Contributor Author

ok, looks like some items for me to look at. the specific tests I added passed but I see there are formatting issues, sorry I hadn't run those, will update

@jensbjorgensen
Copy link
Contributor Author

the tests I've added are passing, seems like the test suite just takes too long to run for some reason?

@jensbjorgensen
Copy link
Contributor Author

the failures in 3.9 are apparently due to asyncio.events._TransProtPair not being defined in python 3.9

@jensbjorgensen
Copy link
Contributor Author

ok! I got test_sourcecode.py fixed up (which I didn't break but ... bonus!), that'd be awesome if one of you maintainers could take a look

Copy link
Member

@fantix fantix left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for adding a new feature!

@jensbjorgensen
Copy link
Contributor Author

please have a look at my latest push and thanks for your time reviewing the PR :-)

@jensbjorgensen
Copy link
Contributor Author

I think everything is lined up now with your comments? Let me know if there's anything else.

@jensbjorgensen
Copy link
Contributor Author

jensbjorgensen commented Jul 27, 2022

ping-ping?

@jensbjorgensen jensbjorgensen requested a review from fantix July 28, 2022 12:34
@fantix
Copy link
Member

fantix commented Aug 13, 2022

Hey sorry for the delay! I talked to Yury and I'm convinced that this new API could have arguably different designs, and it's preferred to be a user-space extension of uvloop, while uvloop is targeting to be a drop-in replacement for asyncio itself only. With that said, we could still have the get_uv_loop_ptr() API accepted (#310) for easier integration with the libuv loop.

@jensbjorgensen
Copy link
Contributor Author

jensbjorgensen commented Aug 15, 2022 via email

@fantix
Copy link
Member

fantix commented Sep 1, 2022

@jensbjorgensen We changed our mind - let's move forward with this PR! I'll probably just push a few commits to make the API private, drop the partially-implemented recursive flag, and merge the rest as it is.

Please feel free to let me know if you have any comments, or create a new PR if this is already merged.

@jensbjorgensen
Copy link
Contributor Author

jensbjorgensen commented Sep 1, 2022 via email

* Move const into fsevent.pyx and import with uvloop.loop * Use libuv const to replace the redeclaration * Hide the _monitor_fs API * Duck-typing the asyncio.Handle API * Dropped recursive flag which is not functional on Linux * Proper UVHandle start and stop for fs_event * Callback with context * Errors in callback won't stop the watch now * Dropped the libuv loop pointer (already implemented in other PR)
This happens when executing chmod on a subdir on Linux
@fantix fantix merged commit 74d381e into MagicStack:master Sep 9, 2022
fantix added a commit that referenced this pull request Sep 13, 2022
This release adds Python 3.11 support, updates bundled libuv to 1.43.0 and fixes a handful of issues. Changes ======= * Expose uv_loop_t pointer for integration with other C-extensions (#310) (by @pranavtbhat in b332eb8 for #310) * Support python 3.11+ (#473) (by @zeroday0619 in 8e42921 for #473) * Expose libuv uv_fs_event functionality (#474) (by @jensbjorgensen @fantix in 74d381e for #474) * Activate debug mode when `-X dev` is used (by @jack1142 in 637a77a) * Expose uv_version() for libuv API compatibility (#491) (by @fantix in 089f6cb for #491) * Fix loop.getaddrinfo() and tests (#495) (by @fantix in 598b16f for #495) * Bump to libuv 1.43.0 (by @fantix in 94e5e53) Fixes ===== * _TransProtPair is no longer defined in asyncio.events (by @jensbjorgensen in fae5f7f) * use a TypeVar for asyncio.BaseProtocol (#478) (by @graingert in 3aacb35 for #478) * Fix segfault in TimerHandle.when() after cleared (by @jensbjorgensen in c39afff for #469) * Avoid self._errpipe_write double close (#466) (by @graingert in 72140d7 for #466) * Fix typo in test (#456) (by @kianmeng in 033d52d for #456) * Fix potential infinite loop (#446) (by @kfur in ada43c0 for #446) * use a stack of self._fds_to_close to prevent double closes (#481) (by @graingert in 3214cf6 for #481) * Fix incorrect main thread id value forking from a thread (#453) (by @horpto @fantix in e7934c8 for #453) * create_subprocess_exec should treat env={} as empty environment (#439) (#454) (by @byllyfish in e04637e for #439) * Queue write only after processing all buffers (#445) (by @jakirkham @fantix in 9c6ecb6 for #445) * Drop Python 3.6 support for thread ident (by @fantix in 9c37930) * bugfix: write to another transport in resume_writing() fails (#498) (by @fantix in d2deffe for #498) Build ===== * Upgrade GitHub Actions (#477) (#480) (by @cclauss in fcbf422 for #477, 1008694 for #480) * typo `same as same` (by @YoSTEALTH in fedba80) * setup.py: allow to override extra_compile_args (#443) (by @giuliobenetti in a130375 for #443) * Drop hack in setup.py in finalize_options (492) (by @fantix in 2f1bc83 for #492) * Fix tests invocation on release CI worklow (#489) (by @ben9923 in d6a2b59 for #489) Documentation ============= * use asyncio.Runner loop_factory on 3.11+ (#472) (by @graingert in 31ba48c for #472) * Fix CI badge in docs, remove remaining Travis CI references from docs (by @Nothing4You in c6901a7) * Fix typo in README (by @monosans in 73d7253)
@fantix fantix mentioned this pull request Sep 13, 2022
fantix added a commit that referenced this pull request Sep 14, 2022
This release adds Python 3.11 support, updates bundled libuv to 1.43.0 and fixes a handful of issues. Changes ======= * Expose uv_loop_t pointer for integration with other C-extensions (#310) (by @pranavtbhat in b332eb8 for #310) * Support python 3.11+ (#473) (by @zeroday0619 in 8e42921 for #473) * Expose libuv uv_fs_event functionality (#474) (by @jensbjorgensen @fantix in 74d381e for #474) * Activate debug mode when `-X dev` is used (by @jack1142 in 637a77a) * Expose uv_version() for libuv API compatibility (#491) (by @fantix in 089f6cb for #491) * Fix loop.getaddrinfo() and tests (#495) (by @fantix in 598b16f for #495) * Bump to libuv 1.43.0 (by @fantix in 94e5e53) Fixes ===== * _TransProtPair is no longer defined in asyncio.events (by @jensbjorgensen in fae5f7f) * use a TypeVar for asyncio.BaseProtocol (#478) (by @graingert in 3aacb35 for #478) * Fix segfault in TimerHandle.when() after cleared (by @jensbjorgensen in c39afff for #469) * Avoid self._errpipe_write double close (#466) (by @graingert in 72140d7 for #466) * Fix typo in test (#456) (by @kianmeng in 033d52d for #456) * Fix potential infinite loop (#446) (by @kfur in ada43c0 for #446) * use a stack of self._fds_to_close to prevent double closes (#481) (by @graingert in 3214cf6 for #481) * Fix incorrect main thread id value forking from a thread (#453) (by @horpto @fantix in e7934c8 for #453) * create_subprocess_exec should treat env={} as empty environment (#439) (#454) (by @byllyfish in e04637e for #439) * Queue write only after processing all buffers (#445) (by @jakirkham @fantix in 9c6ecb6 for #445) * Drop Python 3.6 support for thread ident (by @fantix in 9c37930) * bugfix: write to another transport in resume_writing() fails (#498) (by @fantix in d2deffe for #498) Build ===== * Upgrade GitHub Actions (#477) (#480) (by @cclauss in fcbf422 for #477, 1008694 for #480) * typo `same as same` (by @YoSTEALTH in fedba80) * setup.py: allow to override extra_compile_args (#443) (by @giuliobenetti in a130375 for #443) * Drop hack in setup.py in finalize_options (492) (by @fantix in 2f1bc83 for #492) * Fix tests invocation on release CI worklow (#489) (by @ben9923 in d6a2b59 for #489) Documentation ============= * use asyncio.Runner loop_factory on 3.11+ (#472) (by @graingert in 31ba48c for #472) * Fix CI badge in docs, remove remaining Travis CI references from docs (by @Nothing4You in c6901a7) * Fix typo in README (by @monosans in 73d7253)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

3 participants