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

ENH Parameterise scheduling interval and easily add/remove tasks #75

Open
spmvoss opened this issue Aug 12, 2022 · 6 comments
Open

ENH Parameterise scheduling interval and easily add/remove tasks #75

spmvoss opened this issue Aug 12, 2022 · 6 comments
Labels
documentation Improvements or additions to documentation

Comments

@spmvoss
Copy link

spmvoss commented Aug 12, 2022

I really like the potential of this library. I might be misinterpreting the documentation, but I believe this feature is not (easily) available.

Is your feature request related to a problem? Please describe.
The tasks are not known beforehand/statics. I would like to be able to dynamically create tasks and add them to the scheduler (both before running and while running).

Describe the solution you'd like
Something along the lines of:

app.add_task(name="my_task",interval="every 10 minutes",callback=my_fun)
app.remove_task(name="my_task")

Describe alternatives you've considered
I have tried messing with the Session() and Task() things, as from the documentation it hints that you can create tasks this way.
However, I did not understand from the documentation how this would work other than manipulating existing tasks.

Additional context
I am writing a CLI app where I would like the load the schedule from a configuration file and also allow the user to dynamically add tasks to the schedule.

@spmvoss spmvoss added the enhancement New feature or request label Aug 12, 2022
@Miksus
Copy link
Owner

Miksus commented Aug 12, 2022

This is totally possible to do at the moment but after thinking a bit, I think we should indeed have methods for at least removing tasks. There is an add_task method and a task can be fetched from the session as well using getitem. Good point though, we should make this more intuitive.

The session object (app.session or use the dynamic argument, rocketry.args.Session) contains an attribute .tasks that is a set. This set contains the tasks and it is the single source of truth for them in scheduling session. You are free to add tasks or remove them like:

session = app.session

# Get a task based on its name:
task = session["my_task"]

# Remove task
session.tasks.remove(task)

For adding, it's recommended to use the session.add_task as that checks whether a task already exists with the same name:

from rocketry.tasks import FuncTask
task = FuncTask(func=my_task_func, start_cond="every 10 minutes")
session.add_task(task)

@spmvoss
Copy link
Author

spmvoss commented Aug 12, 2022

Ah you are right, that is indeed exactly what I was looking for.
Thank you providing the additional explanation, that is much appreciated.

I guess that is what the documentation is describing here when it mentions:

@app.task('daily')
def do_things():
    ...

def run_things():
    ...

app.task('daily', func=run_things)
app.task('daily', func_name="main", path="path/to/example.py")
app.task('daily', command='echo "Hello world"')
app.task('daily', code='print("Hello world")')

I did not understand the start condition from that, but I now see that I can pass it indeed with start_cond=.

Indeed it might make sense to have a session.remove_task() to create a streamlined way of adding and removing tasks in the same manner.

@Miksus
Copy link
Owner

Miksus commented Aug 15, 2022

Yep, the first argument is actually the start_cond. Also, it seems I never implemented this app.task('daily', code='print("Hello world")') though the docs say this works. Need to fix that, not sure if it's good to have that implemented this way as there is a danger if someone lets a user pass the arguments of app.task when creating an UI or so.

But back to the issue. Actually, I now added remove_task, and also create_task and add_task to the session instance. This way you should not need to call sometimes the app and sometimes the session to implement these things in your app.

So these should now work

from rocketry.args import Session

@app.task()
def manipulate(session=Session()):
    session.create_task(start_cond="daily", name="a task", func=do_things)
    session.remove_task("a task")

Note that this does not accept the start condition as a positional argument but as a keyword argument. The add_task is useful mostly if you tinker with the task instances which is less common need and it's less documented at the moment. Also I'll probably do some minor changes how tasks store the session in the future. It feels like it's a full-time job to write documentation for this thing: so many aspects and so many layers.

I hope this helps to reach your goal and thanks again for the suggestion!

Does this answer your problem or do you feel I missed something? It seems I forgot to write about these in the documentation. I'll keep the issue open until that's done though.

@spmvoss
Copy link
Author

spmvoss commented Aug 15, 2022

I think that is quite a straight forward and elegant solution. It also indeed covers my question perfectly and I think this will allow me to implement it in the tool I had this envisioned for, thank you!

Yes I can imagine. Sometimes it feels like you can write 20 lines of documentation for every line of code... :)

@Imaclean74
Copy link

I had exactly the same question and use case - but saw the reference to create_task in the release notes and got my use case working before I saw this issue :) But agreed - great to have a section on dynamic ( non decorator based ) task creation.

and great work on the project so far - I'm finding it very useful, especially with the async support, easy integration with FastAPI etc.

@Miksus
Copy link
Owner

Miksus commented Aug 18, 2022

and great work on the project so far - I'm finding it very useful, especially with the async support, easy integration with FastAPI etc.

Thanks a lot, this means a lot to me and comments like yours keep me working on the project :) I think things should be made easy, like FastAPI makes it easy to create APIs, and I have a feeling this will make a lot of other things easy as well. There is a lot this framework could do now and there is a lot this could do in the future.

By the way, I hope you don't mind if I change the label as documentation (as that's the missing part).

@Miksus Miksus added documentation Improvements or additions to documentation and removed enhancement New feature or request labels Aug 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

No branches or pull requests

3 participants