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

[AIRFLOW-2407] import logging for line 68 of kerberos_auth.py #3294

Closed
wants to merge 1 commit into from

Conversation

cclauss
Copy link

@cclauss cclauss commented May 2, 2018

flake8 testing of https://github.com/apache/incubator-airflow on Python 3.6.3

$ flake8 . --count --select=E901,E999,F821,F822,F823 --show-source --statistics

./airflow/contrib/auth/backends/kerberos_auth.py:67:13: F821 undefined name 'logging'
            logging.error('Password validation for principal %s failed %s', user_principal, e)
            ^

Make sure you have checked all steps below.

JIRA

  • My PR addresses the following Airflow JIRA issues and references them in the PR title. For example, "[AIRFLOW-XXX] My Airflow PR"

Description

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

Tests

  • My PR adds the following unit tests OR does not need testing for this extremely good reason:
    This PR fixes flake8 tests which are currently failing.

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"

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 git diff upstream/master -u -- "*.py" | python3 -m flake8 --diff

flake8 testing of https://github.com/apache/incubator-airflow on Python 3.6.3

$ __flake8 . --count --select=E901,E999,F821,F822,F823 --show-source --statistics__
```
./airflow/contrib/auth/backends/kerberos_auth.py:67:13: F821 undefined name 'logging'
            logging.error('Password validation for principal %s failed %s', user_principal, e)
            ^
```
@Fokko
Copy link
Contributor

Fokko commented May 2, 2018

Please open a Jira ticket

@cclauss
Copy link
Author

cclauss commented May 2, 2018

@Fokko What is wrong with AIRFLOW-2407??

@cclauss cclauss changed the title import logging for line 68 of kerberos_auth.py [AIRFLOW-2407] import logging for line 68 of kerberos_auth.py May 2, 2018
@Fokko
Copy link
Contributor

Fokko commented May 2, 2018

You have to prepend the commit with the ticket number, i.e. [AIRFLOW-2407] Import logging for line 68 of kerberos_auth.py. Cheers

@codecov-io
Copy link

Codecov Report

Merging #3294 into master will decrease coverage by <.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3294      +/-   ##
==========================================
- Coverage   75.89%   75.88%   -0.01%     
==========================================
  Files         197      197              
  Lines       14706    14706              
==========================================
- Hits        11161    11160       -1     
- Misses       3545     3546       +1
Impacted Files Coverage Δ
airflow/models.py 86.64% <0%> (-0.05%) ⬇️

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 96d00da...b964970. Read the comment docs.

@kaxil
Copy link
Member

kaxil commented May 2, 2018

I suggest closing all separate PRs #3293 #3294 #3296 #3297 #3298 #3299 and submit a single PR that resolves this issue AIRFLOW-2407
?

@cclauss
Copy link
Author

cclauss commented May 3, 2018

Most of these are in the contrib directory and I have learned on other repos that contributions usually come from different sources and that each contributor may be willing to review/accept on their own code but not on code that they did not contribute. If someone has that confidence then they can merge as many of them as they want.

@kaxil
Copy link
Member

kaxil commented May 3, 2018

@cclauss Don't worry about that. It would be reviewed by one person and directly merged. Please close all the separate PRs and create a single PR.

@cclauss
Copy link
Author

cclauss commented May 3, 2018

Closed in favor of #3307

@cclauss cclauss closed this May 3, 2018
@cclauss cclauss deleted the patch-2 branch May 3, 2018 10:27
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.

4 participants