Skip to content

[AIRFLOW-1321] Fix hidden field key to ignore case#2400

Merged
asfgit merged 1 commit intoapache:masterfrom
sekikn:AIRFLOW-1321
Jul 4, 2017
Merged

[AIRFLOW-1321] Fix hidden field key to ignore case#2400
asfgit merged 1 commit intoapache:masterfrom
sekikn:AIRFLOW-1321

Conversation

@sekikn
Copy link
Contributor

@sekikn sekikn commented Jun 26, 2017

Dear Airflow maintainers,

Please accept this PR. I understand that it will not be reviewed until I have checked off all the steps below!

JIRA

Description

  • Here are some details about my PR, including screenshots of any UI changes:

Webserver has a feature to hide sensitive variable fields,
which key contain specific words. But its matching is
case-sensitive, so "google_api_key" is hidden but
"GOOGLE_API_KEY" is not. This behaviour is not intuitive,
so this PR fixes it to be case-insensitive.

Tests

  • My PR adds the following unit tests OR does not need testing for this extremely good reason:

Added tests/www/test_utils.py.
In addition, I confirmed the feature behaved expectedly as the following screenshot:
v

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
    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"

@codecov-io
Copy link

codecov-io commented Jun 26, 2017

Codecov Report

Merging #2400 into master will decrease coverage by 0.02%.
The diff coverage is 60%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2400      +/-   ##
==========================================
- Coverage   69.26%   69.23%   -0.03%     
==========================================
  Files         146      146              
  Lines       11231    11231              
==========================================
- Hits         7779     7776       -3     
- Misses       3452     3455       +3
Impacted Files Coverage Δ
airflow/www/views.py 67.62% <0%> (ø) ⬆️
airflow/www/utils.py 78.88% <100%> (+0.72%) ⬆️
airflow/jobs.py 74.87% <0%> (-0.41%) ⬇️

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 e870a8e...4620460. Read the comment docs.

@msumit
Copy link
Contributor

msumit commented Jun 26, 2017

@sekikn +1 for moving sensitive field list from views to utils.

Please update your commit message as per the below guidelines, then you are all good:

i. Subject is separated from body by a blank line
ii. Subject is limited to 50 characters

Webserver has a feature to hide sensitive variable fields,
which key contain specific words. But its matching is
case-sensitive, so "google_api_key" is hidden but
"GOOGLE_API_KEY" is not. This behaviour is not intuitive,
so this PR fixes it to be case-insensitive.
@sekikn sekikn changed the title [AIRFLOW-1321] Make hidden variables feature case-insensitive [AIRFLOW-1321] Fix hidden field key to ignore case Jun 27, 2017
@sekikn
Copy link
Contributor Author

sekikn commented Jun 27, 2017

Thanks for the review @msumit! Updated the subject within 50 characters.

@saguziel
Copy link
Contributor

lgtm

@asfgit asfgit merged commit 4620460 into apache:master Jul 4, 2017
asfgit pushed a commit that referenced this pull request Jul 4, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants