Use celery app timezone - #1351
Conversation
|
@mher Let me know if you need something. The main idea here is the use |
auvipy
left a comment
There was a problem hiding this comment.
is it possible to add some tests to verify this?
There was a problem hiding this comment.
Pull Request Overview
This PR improves timezone handling in Flower's task view by using the celery app's configured timezone instead of directly accessing configuration settings. The change ensures proper timezone display for task timestamps whether using explicit timezone settings or local timezone when UTC is disabled.
- Updates timezone detection logic to use
capp.timezone.tzname()instead ofcapp.conf.timezone - Refactors time type variable naming for better clarity
- Adds datetime import to support timezone name resolution
7ba349e to
54b00db
Compare
|
I added UTC and non-UTC tests. copilot broke the PR so I fixed that as well. |
|
I will try to add tests for |
00f9b8d to
2110525
Compare
|
Ok I tested all permutations of |
a8cee47 to
59424c3
Compare
2b2f7ca to
5ac407f
Compare
|
************* Module flower.views.tasks |
af04b60 to
ee83a76
Compare
a3538ea to
01d1b6f
Compare
|
Thanks for running the pipeline and sorry for not checking all permutations locally. The tox.ini is broken. This would fix it: Do you think it is worth fixing? If yes I'll make an issue and a PR. |
|
I have opened a PR based on your suggestion #1451 |
|
Since #1451 is merged, I guess this is now ready for review again? We would love to have this in flower 🙂 |
|
#1132 (comment) shows how to set the local time zone. I don't think we need an additional configuration option. |
|
Setting These two have been causing us trouble for years:
The |
You refer to server-side timezone. The additional |
…ezone to support future changes
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
e58fa73 to
bde7bd0
Compare
|
Rebased on master to
|
If it is a client-side time zone, it should not be configured on the server side. Some clients may want to use the browser time zone, others may not. |
Indeed but Flower still needs the option (--browser-local-time ) to show the client-side time zone. Otherwise you only see sever-side timezone. See the last table of #1351 (comment) for an example. |
|
So in terms of parameters we now have these:
|
Sure but the same thing can be said for We could make both |
Why are you adding the time zone to relative-time tooltips? Clients already know what time zone they're in, so there's no need to keep reminding them. |
Because it isn't necessarily the client's timezone, it can be the Flower timezone or the Celery configured one. Lets say you see
and the tooltip says
The user would need to do some math to figure out that the timezone of the absolute time shown is (America/New_York). So that's why I'm showing it in the tooltip. Hope it is clear, timezone stuff can be tricky and confusing. We fell over this so many times. |


Closes #1132
Changes
browser_local_time(bool) to show the time in the Celery app timezone or the browser local timezone.enable_utc(when Celery optiontimezoneis missing the celery app uses UTC or the system TZ).natural_time=True.Implementation
The task started and received times in the task table are derived from the raw celery events which provide times as number of seconds passed since the epoch.
flower/flower/views/tasks.py
Line 117 in 58136bf
This code takes
timezoneinto account but notenable_utc.https://docs.celeryq.dev/en/stable/userguide/configuration.html#time-and-date-settings
Instead of using the time and date settings from the celery configuration directly, this PR proposes to use the timezone provided by the celery app
https://github.com/celery/celery/blob/8f389997887232500d4aa1a2b0ae0c7320c4c84a/celery/app/base.py#L1360
This allows using an explicit timezone (e.g.
timezone = "Europe/Paris") or the local timezone (enable_utc = False).