-
Notifications
You must be signed in to change notification settings - Fork 3
Supports configuring and running a worker from the CLI #22
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
Conversation
This ended up making me confront the fact that we didn't really deal with how the worker knows which tasks to run. From the client side it's really easy, since you can just put something blindly on the docket and we'll register it for you if it's a function. From the worker side, there's nowhere to really start from. I don't think this is the final form of this, but it's a baby step. This version supports a string path pointing to any iterable of functions you set up, so you can have: ``` my_tasks = [task_a, task_b, task_c] ``` and then register them with `--tasks my.module:my_tasks`. We should really think a lot more about this. Closes #8
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #22 +/- ##
==========================================
Coverage 100.00% 100.00%
==========================================
Files 17 19 +2
Lines 869 1012 +143
Branches 26 38 +12
==========================================
+ Hits 869 1012 +143 ☔ View full report in Codecov by Sentry. |
|
@jakekaplan As I put this together, I realized that most people will probably want to have their own CLI entrypoint to drive the worker anyway. I still think having this |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! This makes sense to me but I agree may not be the final iteration
That's fair. Would you want to take it step further and expose this directly ? async def run() -> None:
async with Docket(name=docket_, url=url) as docket:
for task_path in tasks:
docket.register_collection(task_path)
async with Worker(
docket=docket,
name=name,
prefetch_count=prefetch_count,
redelivery_timeout=redelivery_timeout,
reconnection_delay=reconnection_delay,
) as worker:
if until_finished:
await worker.run_until_finished()
else:
await worker.run_forever() |
|
Okay that slays, let me do that |
| assert "trace" in caplog.text | ||
|
|
||
|
|
||
| def test_worker_command_exposes_all_the_options_of_worker(): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jakekaplan I added this test to make sure we're always exposing all the arguments in the CLI so we can keep things synced up
This ended up making me confront the fact that we didn't really deal
with how the worker knows which tasks to run. From the client side it's
really easy, since you can just put something blindly on the docket and
we'll register it for you if it's a function. From the worker side,
there's nowhere to really start from.
I don't think this is the final form of this, but it's a baby step.
This version supports a string path pointing to any iterable of
functions you set up, so you can have:
and then register them with
--tasks my.module:my_tasks. We shouldreally think a lot more about this.
Closes #8