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

Scheduler update #840

Merged

Conversation

jacopopalumbo01
Copy link
Contributor

Description

Now the StrayCat has the WhiteRabbit property. Added the schedule_jobs. There are some missing things (like the other types of scheduling discussed in #839.

With this update you can schedule a job like follows:

def job(cat):
    cat.send_ws_message(content="White Rabbit: Oh dear! Oh dear! I shall be too late!", msg_type="chat")
    
@hook
def before_cat_sends_message(final_output, cat):    
    cat.white_rabbit.schedule_job(job, seconds=10, cat=cat)
    return final_output

Notice: If you don't need the cat instance in the scheduled job you can just ignore it, now it is handled as a kwarg. This means that you can also pass other attributes to the method. Example:

def job(log_message):
    log.debug(log_message)
    
@hook
def before_cat_sends_message(final_output, cat):    
    cat.white_rabbit.schedule_job(job, seconds=10, log_message="Hello world")
    return final_output

Related to PR #836 discussion and #839

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas

@jacopopalumbo01 jacopopalumbo01 marked this pull request as draft May 28, 2024 20:00
@pieroit
Copy link
Member

pieroit commented May 29, 2024

Looks good to me!

The StrayCat instance passing is now present both in RabbitHole and WhiteRabbite, at one point we could figure out a way to implicitly pass it, whenever the methods are called from a Stray instance. I'll propose for Cat v2, because it requires architectural changes :D

image

@jacopopalumbo01
Copy link
Contributor Author

Update:

  • Added a listener to the scheduler. Now there is a log about the scheduled job behavior after job ends.
  • Added schedule_interval_job() to schedule recurrent jobs

Next thing to do: implement cron type.

Another interesting idea could be a decorator for jobs that do not start inside a hook or a tool.
It should be something like that:

@schedule(date=date)
def func():
     log.info("WhiteRabbit: Hi there!")

What do you think?

P.S.
APScheduler job can also accept an id or a name as parameter. Do you think that we should add those arguments? I'm thinking about managing jobs. We can provide a method to get the scheduled jobs and one to delete them. By having custom ids and names it could be more accessible.

@lucagobbi
Copy link
Collaborator

Really like this! About the @schedule annotation I'm quite neutral, one could instantiate the CheshireCat or directly the WhiteRabbit in his/here plugin code, since they are singleton and schedule the function directly there. But, if it is a requested feature it's for sure less verbose with the annotation.

About the id/name of the scheduled jobs I totally agree: if you start a recurrent task at the moment you cannot delete it at runtime (maybe inserting some logic in the scheduled job itself, but sometimes may not be such a cool idea). Plus, in the future, one could build a plugin(the WhiteRabbitInAction) that creates job, deletes them, pauses them, etc.

@jacopopalumbo01 jacopopalumbo01 marked this pull request as ready for review May 31, 2024 18:20
@jacopopalumbo01
Copy link
Contributor Author

Update: Added ids management, get_jobs, remove_job, schedule_cron_job methods.

@jacopopalumbo01 jacopopalumbo01 changed the title [WIP] Scheduler update Scheduler update May 31, 2024
@pieroit
Copy link
Member

pieroit commented Jun 1, 2024

Update:

  • Added a listener to the scheduler. Now there is a log about the scheduled job behavior after job ends.
  • Added schedule_interval_job() to schedule recurrent jobs

Next thing to do: implement cron type.

Another interesting idea could be a decorator for jobs that do not start inside a hook or a tool. It should be something like that:

@schedule(date=date)
def func():
     log.info("WhiteRabbit: Hi there!")

What do you think?

P.S. APScheduler job can also accept an id or a name as parameter. Do you think that we should add those arguments? I'm thinking about managing jobs. We can provide a method to get the scheduled jobs and one to delete them. By having custom ids and names it could be more accessible.

Awesome!
Let's wait for a dedicated decorator, after there is some usage for this utility we can better understand what is needed

Thanks @jacopopalumbo01 @lucagobbi

@pieroit pieroit merged commit 8e09eed into cheshire-cat-ai:develop Jun 1, 2024
1 check passed
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