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.

Sign up for GitHub

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

Jump to bottom

[AIRFLOW-7048] Allow user to chose timezone to use in UI #8046

Merged
merged 10 commits into from
Apr 2, 2020
Merged

[AIRFLOW-7048] Allow user to chose timezone to use in UI #8046

merged 10 commits into from
Apr 2, 2020

Conversation

ashb
Copy link
Member

@ashb ashb commented Apr 1, 2020 •
edited
Loading

On the home page:

output

Tooltip shown when date is not in UTC:

image

default_timezone from config exposed to UI, and added new setting to allow making that the default timezone.

image

If config timezone is same as local, the local option is not shown:

image

When you haven't yet selected/entered a specific timezone (or you choose the same as one of the ones shown)

image

Things to do left:

  • Mkae FAB's datetime filters convert on timezone change.
  • Bubble up default timezone from airflow.cfg to default option in UI
  • produce gif/screencast of differences.
  • Decide if current "timezoneless" display is right or not.

Issue link: AIRFLOW-7048

Make sure to mark the boxes below before creating PR: [x]

In case of fundamental code change, Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in UPDATING.md.
Read the Pull Request Guidelines for more information.

@boring-cyborg boring-cyborg bot added the area:webserver Webserver related Issues label Apr 1, 2020
dimberman
dimberman requested changes Apr 1, 2020
View reviewed changes
Copy link
Contributor

@dimberman dimberman left a comment

Choose a reason for hiding this comment

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

The code looks clean (based on my limited understanding of front ends), but could you please include some before/after photos? Maybe we can also get Sam or Ryan to take a look as well?

@kaxil kaxil added this to the Airflow 1.10.10 milestone Apr 1, 2020
@ashb ashb requested a review from dimberman April 2, 2020 12:35
@kaxil
Copy link
Member

kaxil commented Apr 2, 2020

Waiting for the CI results

@ashb ashb marked this pull request as ready for review April 2, 2020 13:33
@ashb
Copy link
Member Author

ashb commented Apr 2, 2020

Travis hasn't run the tests yet.

@kaxil
Copy link
Member

kaxil commented Apr 2, 2020

https://travis-ci.org/github/apache/airflow/builds/670149778 - Looks like it is still in Queue

@mik-laj mik-laj closed this Apr 2, 2020
@mik-laj mik-laj reopened this Apr 2, 2020
@ashb
Copy link
Member Author

ashb commented Apr 2, 2020

Tests should pass now, and I updated the timezone.rst doc too

@ashb ashb changed the title [AIRFLOW-7048] Allow user to convert timezone in UI [AIRFLOW-7048] Allow user to chose timezone to use in UI Apr 2, 2020
@kaxil kaxil merged commit cc9b1bc into apache:master Apr 2, 2020
kaxil pushed a commit that referenced this pull request Apr 2, 2020
@ashb @kaxil
[AIRFLOW-7048] Allow user to chose timezone to use in UI (#8046)
f8f0bec 
Co-authored-by: Sam Black <[email protected]>
(cherry picked from commit cc9b1bc)
@kaxil
Copy link
Member

kaxil commented Apr 2, 2020

What the hell happened here? The PR got merged but it is somehow still open. I think the GH outage might have caused this

@kaxil
Copy link
Member

kaxil commented Apr 2, 2020

Good, closing the PR did the trick.

This was referenced Apr 3, 2020
Merged
Closed
ashb added a commit to ashb/airflow that referenced this pull request Apr 6, 2020
@ashb
Fix 500 error in Security screens
532ca11 
PR apache#8046 used our CustomSQLAInterface more widely, and we weren't
correctly implementing the interface right in the constructor, which was
only a problem for models which use Relations, which we don't in
Airflow, but FAB does for it's user/permissions models.
@ashb ashb mentioned this pull request Apr 6, 2020
Merged
5 tasks
kaxil pushed a commit that referenced this pull request Apr 6, 2020
@ashb
Fix 500 error in Security screens (#8165)
0dafdd0 
PR #8046 used our CustomSQLAInterface more widely, and we weren't
correctly implementing the interface right in the constructor, which was
only a problem for models which use Relations, which we don't in
Airflow, but FAB does for it's user/permissions models.
kaxil pushed a commit that referenced this pull request Apr 6, 2020
@ashb @kaxil
Fix 500 error in Security screens (#8165)
515975c 
PR #8046 used our CustomSQLAInterface more widely, and we weren't
correctly implementing the interface right in the constructor, which was
only a problem for models which use Relations, which we don't in
Airflow, but FAB does for it's user/permissions models.

cherry-picked from 0dafdd0
@bcjonescbt
Copy link

I upgraded to version 1.10.10 but I don't see the new GUI tooltips in the upper right. I added the default_ui_timezone setting to my config file, but still no luck. It looks the same as it ever did (just showing UTC). Do I have to burn down my database to get this new feature?

@bcjonescbt
Copy link

My entire header row is different than the picture in this PR. I don't have the pretty icons, and I don't have a user on the right. The menu items don't match either. Is there some config setting that I need to keep up with to get these new features?

airflow_GUI

@kaxil
Copy link
Member

kaxil commented Apr 9, 2020

My entire header row is different than the picture in this PR. I don't have the pretty icons, and I don't have a user on the right. The menu items don't match either. Is there some config setting that I need to keep up with to get these new features?

airflow_GUI

Please enable RBAC UI - https://airflow.apache.org/blog/airflow-1.10.8-1.10.9/#use-airflow-rbac-ui to use that feature.

@bcjonescbt
Copy link

Ah - it's a brave new world. Thanks for the pointer.

@vanducng
Copy link
Contributor

Im waiting this feature quite so long (though it would be available in v2), thank you so much @ashb @kaxil
image

@alexbegg alexbegg mentioned this pull request Aug 20, 2020
Closed
@ashb ashb deleted the ui-timezone-aware branch March 25, 2021 13:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ryanahamilton ryanahamilton ryanahamilton left review comments

@dimberman dimberman dimberman approved these changes

@kaxil kaxil kaxil approved these changes

Assignees
No one assigned
Labels
area:webserver Webserver related Issues
Projects
None yet
Milestone
Airflow 1.10.10
Development

Successfully merging this pull request may close these issues.
7 participants
@ashb @kaxil @bcjonescbt @vanducng @ryanahamilton @dimberman @mik-laj