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

feat: adds @cron task decorator #56

Open
wants to merge 14 commits into
base: main
Choose a base branch
from
Open

feat: adds @cron task decorator #56

wants to merge 14 commits into from

Conversation

mikeshultz
Copy link
Contributor

@mikeshultz mikeshultz commented Feb 23, 2024

What I did

Adds the @cron task decorator so we can execute arbitrary tasks according to a crontab schedule.

How I did it

Adds an event loop to the runner that checks if cron tasks should be executes. If true, it will kiq the tasks off.

How to verify it

@app.cron("*/5 * * * *")
def every_five_minutes():
    return {"crontime": datetime.utcnow()}

Checklist

  • Passes all linting checks (pre-commit and CI jobs)
  • New test cases have been added and are passing
  • Documentation has been updated
  • PR title follows Conventional Commit standard (will be automatically included in the changelog)

@mikeshultz mikeshultz added the enhancement New feature or request label Feb 23, 2024
@mikeshultz mikeshultz self-assigned this Feb 23, 2024
@mikeshultz mikeshultz marked this pull request as ready for review February 23, 2024 00:31
@mikeshultz mikeshultz requested a review from fubuloubu February 23, 2024 00:31
silverback/types.py Outdated Show resolved Hide resolved
silverback/types.py Outdated Show resolved Hide resolved
Execute scheduled cron tasks when their sequence comes up.
"""
while True:
await asyncio.sleep(CRON_TICK)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what happens if the cronjob is scheduled to be less than 15 seconds apart?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cron has a resolution of 1 minute.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reduced his to 5 just to be a little more accurate

Can do like a drift sort of logic to make it loosely align with :00 +/- 1 sec (as clock drift may occur)

silverback/types.py Outdated Show resolved Hide resolved
@fubuloubu fubuloubu marked this pull request as draft June 1, 2024 22:07
@fubuloubu
Copy link
Member

fubuloubu commented Jun 1, 2024

Upgraded this, but need to figure out why crons aren't triggering


Edit: figured out it was due to an internal failure 7a93ab3, which weren't getting displayed properly b77e718

@fubuloubu fubuloubu marked this pull request as ready for review June 4, 2024 01:09
@fubuloubu
Copy link
Member

p.s. this website had some good test cases: https://crontab.guru/

You can do that with the `@cron` task decorator.

```python
@app.cron("* */1 * * *")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I should actually test this...

Copy link
Contributor Author

@mikeshultz mikeshultz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't even approve this because it's technically mine. May as well just have created a replacement PR for this one.

if current_time.second < CRON_CHECK_SECONDS:
# Print out current time every minute
logger.info(f"Current Time: {current_time}")
# NOTE: In the absence of any cron jobs, we still print out the current time
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion: make this debug. bit verbose and not really useful unless you're debugging.

Copy link
Member

@fubuloubu fubuloubu Aug 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was actually thinking this could be really useful to see for endusers, because they would have some live logging being created in scenarios where they are solely doing event log monitoring and nothing is really happening. Also very useful to know what the bot's local time is vs. the blockchain's, to see if there is some drift occuring.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants