Skip to content

[AIRFLOW-2799] Fix filtering UI objects by datetime#4061

Merged
Fokko merged 1 commit intoapache:masterfrom
ashb:www-filter-utcdatetimes
Nov 8, 2018
Merged

[AIRFLOW-2799] Fix filtering UI objects by datetime#4061
Fokko merged 1 commit intoapache:masterfrom
ashb:www-filter-utcdatetimes

Conversation

@ashb
Copy link
Member

@ashb ashb commented Oct 17, 2018

Make sure you have checked all steps below.

Jira

Description

  • Our conversion to Timezone-aware date times in 1.10 missed this case -
    any filter on a date time would blow up with naive datetime is disallowed - this makes our datetime filters timezone aware now (they
    respect the default_timezone, and they accept timezones in input even
    though the UI won't let you craft those.)

Tests

  • added a simple test to www.test_views and www_rbac.test_views

Commits

  • My commits all reference Jira issues in their subject lines, and I have squashed multiple commits if they address the same issue. In addition, my commits follow the guidelines from "How to write a good git commit message":
    1. Subject is separated from body by a blank line
    2. Subject is limited to 50 characters (not including Jira issue reference)
    3. Subject does not end with a period
    4. Subject uses the imperative mood ("add", not "adding")
    5. Body wraps at 72 characters
    6. Body explains "what" and "why", not "how"

Documentation

  • In case of new functionality, my PR adds documentation that describes how to use it.
    • When adding new operators/hooks/sensors, the autoclass documentation generation needs to be added.

Code Quality

  • Passes flake8

@ashb
Copy link
Member Author

ashb commented Oct 17, 2018

@jgao54 @mistercrunch If either of you have time to check on the FAB changes I made here that would be good - it works, but I'm not 100% sure its the best way.

Also somewhat annoyingly there seems to be no easy way in a filter to validate the input, so right now if you give an invalid DateTime (one that cannot be parsed) it will throw a 500 instead of a nice message. There doesn't seem to be a ValidationError or anything that I could easily throw to have this handled nicely that I could see from FAB's docs.

@ashb ashb mentioned this pull request Oct 17, 2018
Closed
3 tasks
@ashb ashb force-pushed the www-filter-utcdatetimes branch from 2a58318 to 06f72d2 Compare October 17, 2018 18:48
@codecov-io
Copy link

codecov-io commented Oct 17, 2018 •
edited
Loading

Codecov Report

Merging #4061 into master will increase coverage by 0.14%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4061      +/-   ##
==========================================
+ Coverage   75.91%   76.05%   +0.14%     
==========================================
  Files         199      199              
  Lines       15961    15999      +38     
==========================================
+ Hits        12116    12168      +52     
+ Misses       3845     3831      -14
Impacted Files Coverage Δ
airflow/www_rbac/views.py 72.49% <100%> (-0.06%) ⬇️
airflow/www/utils.py 90.27% <100%> (+0.92%) ⬆️
airflow/www_rbac/utils.py 74.09% <100%> (+5.14%) ⬆️
airflow/models.py 91.99% <0%> (+0.04%) ⬆️
airflow/www/views.py 69.46% <0%> (+0.55%) ⬆️
airflow/www_rbac/security.py 92.61% <0%> (+1.34%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d176fa5...4e5bee9. Read the comment docs.

ebrard
ebrard reviewed Oct 17, 2018
View reviewed changes
@ashb ashb force-pushed the www-filter-utcdatetimes branch 2 times, most recently from f2694b0 to 4e5bee9 Compare October 18, 2018 15:06
@ashb
Copy link
Member Author

ashb commented Oct 18, 2018

@ebrard Tests are green now (helps if I actually hit enter on the terminal where I wrote git push :) ) if you could give it another look over?

@ashb
Copy link
Member Author

ashb commented Oct 26, 2018

@bolkedebruin PTAL

@kaxil kaxil requested review from Fokko and feng-tao October 26, 2018 15:34
@kaxil
Copy link
Member

kaxil commented Oct 26, 2018

Hi, @jgao54 can you review this one too please?

bolkedebruin
bolkedebruin reviewed Oct 31, 2018
View reviewed changes
@ashb
[AIRFLOW-2799] Fix filtering UI objects by datetime
b31db26 
Our conversion to Timezone-aware date times in 1.10 missed this case -
any filter on a date time would blow up with `naive datetime is
disallowed` - this makes our datetime filters timezone aware now (they
respect the default_timezone, and they accept timezones in input even
though the UI won't let you craft those.)

I manually tested this by changing query parameters - a value of
`2018-10-30+01%3A05%3A00%2B01:00` matched against an execution date of
00:05:00+00:00
@ashb ashb force-pushed the www-filter-utcdatetimes branch from 4e5bee9 to b31db26 Compare October 31, 2018 21:08
@Fokko Fokko merged commit 4083a8f into apache:master Nov 8, 2018
ashb added a commit that referenced this pull request Nov 8, 2018
@ashb
[AIRFLOW-2799] Fix filtering UI objects by datetime (#4061)
412985d 
Our conversion to Timezone-aware date times in 1.10 missed this case -
any filter on a date time would blow up with `naive datetime is
disallowed` - this makes our datetime filters timezone aware now (they
respect the default_timezone, and they accept timezones in input even
though the UI won't let you craft those.)

I manually tested this by changing query parameters - a value of
`2018-10-30+01%3A05%3A00%2B01:00` matched against an execution date of
00:05:00+00:00
wyndhblb pushed a commit to asappinc/incubator-airflow that referenced this pull request Nov 9, 2018
@ashb @wyndhblb
[AIRFLOW-2799] Fix filtering UI objects by datetime (apache#4061)
5018a15 
Our conversion to Timezone-aware date times in 1.10 missed this case -
any filter on a date time would blow up with `naive datetime is
disallowed` - this makes our datetime filters timezone aware now (they
respect the default_timezone, and they accept timezones in input even
though the UI won't let you craft those.)

I manually tested this by changing query parameters - a value of
`2018-10-30+01%3A05%3A00%2B01:00` matched against an execution date of
00:05:00+00:00
galak75 pushed a commit to VilledeMontreal/incubator-airflow that referenced this pull request Nov 23, 2018
@ashb @galak75
[AIRFLOW-2799] Fix filtering UI objects by datetime (apache#4061)
fa82564 
Our conversion to Timezone-aware date times in 1.10 missed this case -
any filter on a date time would blow up with `naive datetime is
disallowed` - this makes our datetime filters timezone aware now (they
respect the default_timezone, and they accept timezones in input even
though the UI won't let you craft those.)

I manually tested this by changing query parameters - a value of
`2018-10-30+01%3A05%3A00%2B01:00` matched against an execution date of
00:05:00+00:00
tmiller-msft pushed a commit to cse-airflow/incubator-airflow that referenced this pull request Nov 27, 2018
@ashb @tmiller-msft
[AIRFLOW-2799] Fix filtering UI objects by datetime (apache#4061)
7d4b3d2 
Our conversion to Timezone-aware date times in 1.10 missed this case -
any filter on a date time would blow up with `naive datetime is
disallowed` - this makes our datetime filters timezone aware now (they
respect the default_timezone, and they accept timezones in input even
though the UI won't let you craft those.)

I manually tested this by changing query parameters - a value of
`2018-10-30+01%3A05%3A00%2B01:00` matched against an execution date of
00:05:00+00:00
elizabethhalper pushed a commit to cse-airflow/incubator-airflow that referenced this pull request Dec 7, 2018
@ashb @elizabethhalper
[AIRFLOW-2799] Fix filtering UI objects by datetime (apache#4061)
295cc4a 
Our conversion to Timezone-aware date times in 1.10 missed this case -
any filter on a date time would blow up with `naive datetime is
disallowed` - this makes our datetime filters timezone aware now (they
respect the default_timezone, and they accept timezones in input even
though the UI won't let you craft those.)

I manually tested this by changing query parameters - a value of
`2018-10-30+01%3A05%3A00%2B01:00` matched against an execution date of
00:05:00+00:00
aliceabe pushed a commit to aliceabe/incubator-airflow that referenced this pull request Jan 3, 2019
@ashb
[AIRFLOW-2799] Fix filtering UI objects by datetime (apache#4061)
6ce1b9c 
Our conversion to Timezone-aware date times in 1.10 missed this case -
any filter on a date time would blow up with `naive datetime is
disallowed` - this makes our datetime filters timezone aware now (they
respect the default_timezone, and they accept timezones in input even
though the UI won't let you craft those.)

I manually tested this by changing query parameters - a value of
`2018-10-30+01%3A05%3A00%2B01:00` matched against an execution date of
00:05:00+00:00
cfei18 pushed a commit to cfei18/incubator-airflow that referenced this pull request Jan 23, 2019
@ashb
Fix filtering UI objects by datetime (apache#4061)
1f49945 
Our conversion to Timezone-aware date times in 1.10 missed this case -
any filter on a date time would blow up with `naive datetime is
disallowed` - this makes our datetime filters timezone aware now (they
respect the default_timezone, and they accept timezones in input even
though the UI won't let you craft those.)

I manually tested this by changing query parameters - a value of
`2018-10-30+01%3A05%3A00%2B01:00` matched against an execution date of
00:05:00+00:00
@ashb ashb deleted the www-filter-utcdatetimes branch April 4, 2019 14:52
wmorris75 pushed a commit to modmed-external/incubator-airflow that referenced this pull request Jul 29, 2019
@ashb
[AIRFLOW-2799] Fix filtering UI objects by datetime (apache#4061)
efdb41f 
Our conversion to Timezone-aware date times in 1.10 missed this case -
any filter on a date time would blow up with `naive datetime is
disallowed` - this makes our datetime filters timezone aware now (they
respect the default_timezone, and they accept timezones in input even
though the UI won't let you craft those.)

I manually tested this by changing query parameters - a value of
`2018-10-30+01%3A05%3A00%2B01:00` matched against an execution date of
00:05:00+00:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bolkedebruin bolkedebruin bolkedebruin left review comments

@Fokko Fokko Fokko approved these changes

@mistercrunch mistercrunch Awaiting requested review from mistercrunch

@jgao54 jgao54 Awaiting requested review from jgao54

@feng-tao feng-tao Awaiting requested review from feng-tao

+1 more reviewer

@ebrard ebrard ebrard left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@ashb @codecov-io @kaxil @Fokko @bolkedebruin @ebrard