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.

Sign up for GitHub

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

Jump to bottom

[23.1] Exclude on_opened and on_closed from watcher events #16850

Merged
merged 1 commit into from
Oct 13, 2023
Merged

[23.1] Exclude on_opened and on_closed from watcher events #16850

merged 1 commit into from
Oct 13, 2023

Conversation

mvdbeek
Copy link
Member

@mvdbeek mvdbeek commented Oct 13, 2023 •
edited by nsoranzo
Loading

We're not interested in these. on_opened in particular was introduced in gorakhargosh/watchdog#941 and then partially reverted in
gorakhargosh/watchdog@25a2d1f, but that doesn't work for us.

Should fix #16840

How to test the changes?

(Select all options that apply)

  • I've included appropriate automated tests.
  • This is a refactoring of components with existing test coverage.
  • Instructions for manual testing are as follows:
    1. [add testing steps and prerequisites here if you didn't write automated tests covering all your changes]

License

  • I agree to license these and all my past contributions to the core galaxy codebase under the MIT license.

@mvdbeek
Exclude on_opened and on_closed from watcher events
e9bc593 
We're not interested in these. `on_opened` in particular was introduced
in gorakhargosh/watchdog#941 and then partially
reverted in
gorakhargosh/watchdog@25a2d1f,
but that doesn't work for us.

Should fix galaxyproject#16840
@github-actions github-actions bot added this to the 23.2 milestone Oct 13, 2023
@nsoranzo
Copy link
Member

Possible alternative approach: https://github.com/Bogdanp/dramatiq/pull/552/files#diff-0c44d7790da07fa8fd6336bde724085be4376248b69db0cf1f7905bf52e6d6ff

Not sure which one is better.

@mvdbeek
Copy link
Member Author

mvdbeek commented Oct 13, 2023

Not sure which one is better.

I prefer this one since it seems a little more API-ish and typeable (not that we do that currently ...).

@mvdbeek mvdbeek merged commit 6659840 into galaxyproject:release_23.1 Oct 13, 2023
37 of 39 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nsoranzo nsoranzo nsoranzo approved these changes

Assignees
No one assigned
Labels
area/backend kind/bug
Projects
None yet
Milestone
23.2
Development

Successfully merging this pull request may close these issues.
2 participants
@mvdbeek @nsoranzo