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

Ability to customize timezone used by Celery [feature request] #183

Open
bgottsch opened this issue Mar 10, 2023 · 3 comments
Open

Ability to customize timezone used by Celery [feature request] #183

bgottsch opened this issue Mar 10, 2023 · 3 comments
Assignees
Labels
enhancement New feature or request

Comments

@bgottsch
Copy link

bgottsch commented Mar 10, 2023

Hi everyone,

I found no way to customize the timezone used by Celery and would like to propose an improvement.

This would be very useful, since the periodic: crontab feature defaults to UTC timezone, making the cron definition "harder to understand".

Explaining the functionality in Celery

By Celery 5.2.7 documentation, timezones should be implemented by configuring the following parameters when creating the Celery app: enable_utc and timezone.

As explained there, enable_utc defaults to true, and if true uses UTC as the default timezone for Celery. If set to false, Celery uses the system's timezone (in case of docker / docker-compose, it should be easily set by the TZ environment variable).

Likewise, when timezone is set to an appropriate pytz value, Celery uses it.

Changes to celery-director

  • Add an optional environment variable such as DIRECTOR_TZ or DIRECTOR_ENABLE_UTC
  • Remove webserver UTC adjustment
  • Modify file by changing the lines:
...
# Celery configuration
  self.CELERY_CONF = {
      "task_always_eager": False,
      "broker_url": env.str("DIRECTOR_BROKER_URI", "redis://localhost:6379/0"),
      "result_backend": env.str(
          "DIRECTOR_RESULT_BACKEND_URI", "redis://localhost:6379/1"
      ),
      "broker_transport_options": {"master_name": "director"},
  }
...

About the pull request

I'm able and willing to create this modification and help as needed.

What I need is guidance on whether or not this should be done. And if so, how it should be done. Both enable_utc and timezone allow for the timezone to be customized, but I have doubts about which one is better to use.

My initial idea was to create only DIRECTOR_TZ and modify the code to check if it is set. And if not, default to UTC (default value in Celery).

Anyways,

Thanks for this amazing project!

Best regards

@ncrocfer
Copy link
Member

Hi @bgottsch ,

Indeed this is a very good idea ! I will check at the beginning of next week the enable_utc and timezone options I just discovered, and I will give you my feedback ;)

Thanks your your help on this project 👍

@ncrocfer ncrocfer added the enhancement New feature or request label Mar 10, 2023
@ncrocfer ncrocfer self-assigned this Mar 10, 2023
@bgottsch
Copy link
Author

bgottsch commented Mar 10, 2023

Hi @ncrocfer ,

Thanks for the quick reply.

Update on the implementation

I created a fork of this repo and a dev/timezone branch. For now, I'm ignoring enable_utc and only using timezone. A new optional environment variable called DIRECTOR_TZ is created.

Preview of code changes to settings.py:

...
HIDDEN_CONFIG = [
    ...
    "DIRECTOR_TZ",
]
...
# Celery configuration
self.CELERY_CONF = {
    "task_always_eager": False,
    "broker_url": env.str("DIRECTOR_BROKER_URI", "redis://localhost:6379/0"),
    "result_backend": env.str(
        "DIRECTOR_RESULT_BACKEND_URI", "redis://localhost:6379/1"
    ),
    "broker_transport_options": {"master_name": "director"},
    "timezone": env.str("DIRECTOR_TZ", "UTC"),    # <------ ADDED
}
...

I'm trying to keep it as simple as possible.

Results

By setting DIRECTOR_TZ to something valid (eg. "America/Sao_Paulo"), all services but Flower work with the new timezone. To get Flower to work, I had to change the startup command for it:

Preview of code changes to commands/celery.py:

...
@celery.command(name="flower", context_settings=dict(ignore_unknown_options=True))
@click.argument("flower_args", nargs=-1, type=click.UNPROCESSED)
@pass_ctx
def flower(ctx, flower_args):
    """Start the flower instance"""
    broker = ctx.app.config["CELERY_CONF"]["broker_url"]
    args = [
        "celery",
        "-A",                    # <----- ADDED
        "director._auto:cel",    # <----- ADDED
        # "-b",                  # <----- REMOVED
        # broker,                # <----- REMOVED
        "flower"
    ]
    args += list(flower_args)
    os.execvp(args[0], args)
...

Is there a reason for Flower not using the Celery app instance? As per pull request, there is no mention of why it is different than beat or worker.

Misc

As a check, the webserver UI displays the timezone correctly, as per user local timezone. The implementation below is working as expected:

File static/script.js (no changes):

...
Vue.filter("formatDate", function (value) {
  if (value) {
    return moment.utc(value).local().format("YYYY-MM-DD HH:mm:ss Z");
  }
});
...

@bgottsch
Copy link
Author

Here's a merge comparison for better readability: merge with dev/timezone

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

No branches or pull requests

2 participants