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

Worker updates #2938

Merged
merged 21 commits into from
Jul 23, 2023
Merged

Worker updates #2938

merged 21 commits into from
Jul 23, 2023

Conversation

davep
Copy link
Contributor

@davep davep commented Jul 13, 2023 •
edited
Loading

Seeks to implement #2928. In doing so the PR:

  • Adds a thread keyword argument to work.
  • Adds code that checks if the decorated method is async and throws an error if thread=True.
  • Updates DirectoryTree to mark the thread worker as a thread.
  • Updates the thread worker example code to mark the thread worker as a thread.
  • Updates the worker guide to highlight the above checks.
  • Adds the ability to run coroutine-based work as a thread worker.
  • Adds the ability to run awaitable work as a thread worker.
  • Updates the worker-based unit tests to take the above changes into account.
  • Moves the worker-based unit tests into their own subdirectory for easier focused testing.
  • Adds a CHANGELOG entry to point out this is a breaking change.

Also rolled in a couple of tweaks to the overloads for the work decorator to address #2946.

@davep davep self-assigned this Jul 13, 2023
@davep davep added enhancement New feature or request Task labels Jul 13, 2023
@davep davep linked an issue Jul 13, 2023 that may be closed by this pull request
Worker interface #2928
Closed
davep added 9 commits July 13, 2023 14:33
@davep
Update the CHANGELOG
2dfb44f
@davep
Typo fix
e20137a
@davep
Add tests for non-thread-non-async-worker
50657ba
@davep
Allow threaded async
c6cd185 
Originally the request had been communicated as the following would be
errors:

- Non-async method, non-thread
- Async method, thread

As of
Textualize#2928 (comment)
it's been decided that the scope of the PR should be expanded to not only
guard against the first case, but also to extend the way the worker API
works to do the right thing in the second case.

This commit backs out the detection of the second case and flagging it as an
error.
@davep
Add support for running an async method in a thread worker
d458324
@davep
Be explicit about how someone should decorate a non-async worker
baa40e3
@davep
Merge branch 'main' into unfootgun-worker-thread
6a0ad92
@davep
Move worker tests into their own sub-directory
6638958 
This makes it far easier to run this subset of tests all at once.
@davep
Tidy up the worker running code
8e135e7 
Simplify a wee bit, and also tidy up some of the docstirngs.
@davep davep changed the title Make it harder to accidentally create a thread worker Worker updates Jul 18, 2023
@davep davep mentioned this pull request Jul 18, 2023
Closed
@davep davep linked an issue Jul 18, 2023 that may be closed by this pull request
Revisit and update the typing for the work decorator #2946
Closed
@davep davep marked this pull request as ready for review July 18, 2023 09:58
@davep davep merged commit f78ffe0 into Textualize:main Jul 23, 2023
@davep davep deleted the unfootgun-worker-thread branch July 23, 2023 15:32
@adeemm adeemm mentioned this pull request Aug 7, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@willmcgugan willmcgugan willmcgugan approved these changes

@rodrigogiraoserrao rodrigogiraoserrao Awaiting requested review from rodrigogiraoserrao

@darrenburns darrenburns Awaiting requested review from darrenburns

Assignees

@davep davep

Labels
enhancement New feature or request Task
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Revisit and update the typing for the work decorator Worker interface
2 participants
@davep @willmcgugan