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

Fix issue #2135 #2150

Conversation

prestonhale
Copy link

@prestonhale prestonhale commented Aug 5, 2022

Fully fixes the issue in #2135 for me. Not sure what the purpose of maintaining the user_dispatcher was in the first place so take this with a grain of salt. If you do want to dig deeper the issue seems to be the _dispatch generator.

@cyberw
Copy link
Collaborator

cyberw commented Aug 7, 2022

It seems to break a test, but maybe the test is wrong?

@cyberw cyberw changed the title Fix issue 2135 Fix issue #2135 Aug 7, 2022
@mboutet
Copy link
Contributor

mboutet commented Aug 7, 2022

@cyberw see #2152 for the appropriate fix and reproducing test.

As for test_stop_timeout_with_ramp_down, it is an odd test indeed. Even when it was introduced in ded61e0, the test name doesn't make much sense. It does not strike me as test for validating the stop_timeout as the name implies. If it was the case, I would expect the task to sleep for more than two seconds.

Moreover, the docstring I added in d9bb87f is outdated and misleading. I'll remove it in a separate PR.

However, I'm not sure it should be removed as the test itself makes sense (apart from its name).

@cyberw
Copy link
Collaborator

cyberw commented Aug 7, 2022

This should be closed then, I suppose.

@cyberw cyberw closed this Aug 7, 2022
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