Miksus / rocketry

Modern scheduling library for Python
https://rocketry.readthedocs.io
MIT License
3.25k stars 105 forks source link

BUG - setting `task_execution` to main does not seem to work. #125

Closed djnnvx closed 1 year ago

djnnvx commented 1 year ago

Describe the bug I want to have periodic tasks running in my main thread, but it does not seem to work. if initialized with nothing, rocketry will:

  1. attempt to run the tasks as async
  2. fail on the tasks because they cannot be async

this is ok because the log messages are helpful in suggesting that I should initialize the Rocketry app with `Rocketry(task_execution='async') or something.

Problem is that when I initialize with (task_execution='main'), the startup fails. It seems that the log message is not up to date because docs suggest using ```python config={ 'task_execution': 'main' }


but this has apparently no affect on the configuration (see logs below)

**To Reproduce**
Here is the code I've used to initialize the tasks:
```python
from api.core.management.jobs import something_else
from api.core.management.jobs import something

from django.core.management.base import BaseCommand
import logging

from rocketry import Rocketry
from rocketry.conds import daily

from rocketry import Rocketry, Session
from rocketry.conds import daily, every
from rocketry.log import MinimalRecord
from redbird.repos import CSVFileRepo

class JobLogger(MinimalRecord):
    exc_text: Optional[str] = Field(description="Exception text")

repo = CSVFileRepo(filename="/tmp/api-logs/jobs.csv", model=JobLogger)

scheduler : Rocketry = Rocketry(
        logger_repo=repo,
        config={
            'task_execution': 'main'
        }
    )

# i've attempted to set scheduler.session.config.task_execution to `main` here, but to no avail

@scheduler.task(daily.at("00:10"), execution='main')
def do_something() -> None:
    logging.getLogger(__name__).info("Running do_something()")
    something.run()

@scheduler.task(daily.at("23:55"), execution='main')
def do_something_else() -> None:
    logging.getLogger(__name__).info("Running do_something_else()")
    something_else.run()

class Command(BaseCommand):
    help = "Setup the periodic jobs runner"

    def handle(self, *args, **options):
          scheduler.run()

Expected behavior Run the tasks in my main thread and not have the app crash on startup.

Screenshots

/usr/local/lib/python3.10/site-packages/rocketry/session.py:73: FutureWarning: Default execution will be changed to 'async'. To suppress this warning, specify task_execution, ie. Rocketry(task_execution='async') 

and then later on, when the job starts:

Traceback (most recent call last):
  File ""/usr/local/lib/python3.10/site-packages/rocketry/core/task.py"", line 536, in _run_as_async
    output = await self.execute(**params)
  File ""/usr/local/lib/python3.10/site-packages/rocketry/tasks/func.py"", line 234, in execute
    output = func(**params)
  File ""/server/api/core/management/commands/initjobs.py"", line 40, in do_something
    do_something.run()
  File ""/server/api/core/management/jobs/do_something.py"", line 46, in run
    for value in fetched_from_database:
  File ""/usr/local/lib/python3.10/site-packages/django/db/models/query.py"", line 394, in __iter__
    self._fetch_all()
  File ""/usr/local/lib/python3.10/site-packages/django/db/models/query.py"", line 1866, in _fetch_all
    self._result_cache = list(self._iterable_class(self))
  File ""/usr/local/lib/python3.10/site-packages/django/db/models/query.py"", line 87, in __iter__
    results = compiler.execute_sql(
  File ""/usr/local/lib/python3.10/site-packages/django/db/models/sql/compiler.py"", line 1393, in execute_sql
    cursor = self.connection.cursor()
  File ""/usr/local/lib/python3.10/site-packages/django/utils/asyncio.py"", line 24, in inner
    raise SynchronousOnlyOperation(message)
django.core.exceptions.SynchronousOnlyOperation: You cannot call this from an async context - use a thread or sync_to_async."

Desktop (please complete the following information): Python version: 3.10.7 Rocketry version: 2.4.1 Django version: 4.1

Additional context N/A

djnnvx commented 1 year ago

I might have made a silly mistake when implementing this, but I think that in the very least the log message/relevant docs should be updated because this is very confusing for my tiny brain :(

Please let me know if you find something

Regards

djnnvx commented 1 year ago

Just noticed that a new release is available and fixes this! Oh my god!!

Sorry for the disturbance, please let me know if you want me to update the docs/logs in the relevant places, I think I can manage to find some time for that.

Miksus commented 1 year ago

No problem! I respect the time you invested to explain, elaborate and investigate the issue, even though it seemed not to be an issue.

What comes to docs, I think the time subpackage (in rocketry/time/ and rocketry/core/time) could be documented but that's probably not an easy place to get started with (as it completely lacks documentation and requires quite a lot reading code).

But the cookbook/examples could be extended. Also, I think the condition handbook is perhaps not in the best state. Originally the string syntax was the preferred one and then I created the condition API which should now be preferred (this is what you used in the example above). Perhaps the docs could also be formatted in a way that the condition API is first explained and then as a subsection the string syntax. Also, this I think is in the wrong place: https://rocketry.readthedocs.io/en/stable/condition_syntax/index.html. Could be in the handbook.

But more little things to update in the docs:

I'm very much open for contribution.

djnnvx commented 1 year ago

I could start by adding a setup with Gunicorn & the Django Rest Framework, as this is what I'm using right now. My setup is mostly based on the solution proposed in #76, but with some caveats.

Miksus commented 1 year ago

That would be awesome! I'm personally not qualified for writing the documentation regarding how to integrate Rocketry with Django (as I haven't used Django myself) but as it is so used, documentation regarding Rocketry+Django would extend Rocketry's audience quite a lot.

djnnvx commented 1 year ago

Awesome, will try to work on it by the end of the week. I have exams this week so I might be busy (& therefore a bit late) but i'll do my best.

Regards