Skip to content

[AIRFLOW-2216] Use profile for AWS hook if S3 config file provided in aws_default connection extra parameters #3172

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
Closed
lorenanicole wants to merge 1 commit into from
Closed

[AIRFLOW-2216] Use profile for AWS hook if S3 config file provided in aws_default connection extra parameters #3172

lorenanicole wants to merge 1 commit into from

Conversation

@lorenanicole
Copy link

@lorenanicole lorenanicole commented Mar 28, 2018 •
edited
Loading

[AIRFLOW-2216] Use profile for AWS hook if S3 config file provided in aws_default connection extra parameters; add test to validate profile set

Make sure you have checked all steps below.

JIRA

Description

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

NO UI CHANGES MADE

Tests

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

UNIT TEST ADDED

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"

  • Passes git diff upstream/master -u -- "*.py" | flake8 --diff

@Fokko
Copy link
Contributor

Fokko commented Mar 28, 2018

@lorenanicole The tests seem to fail. Also we've introduced a Flake8 test on the git diff's to improve the quality of code. Could you fix these violations as well? Cheers

@lorenanicole
Copy link
Author

lorenanicole commented Mar 29, 2018

@Fokko I'll look at the Travis-CI. Thanks!

@lorenanicole
[AIRFLOW-2216] Use profile for AWS hook if S3 config file provided in…
0cc0cda 
… aws_default connection extra parameters; add test to validate profile set
@lorenanicole lorenanicole force-pushed the add-profile-param-for-awshook branch from 05faba9 to 0cc0cda Compare March 29, 2018 17:06
@lorenanicole
Copy link
Author

lorenanicole commented Mar 29, 2018 •
edited
Loading

@Fokko the remaining issues from flake8 in the files my PR touches are preexisting therefore I won't be adjusting them. For the record these are what I see

# File Code Change In
./airflow/contrib/hooks/aws_hook.py:70: W802 undefined name 'logging'
./airflow/contrib/hooks/aws_hook.py:110:80: E501 line too long (83 > 79 characters)
# File Test In
./tests/contrib/hooks/test_aws_hook.py:30: W801 redefinition of unused 'mock' from line 28
./tests/contrib/hooks/test_aws_hook.py:142:80: E501 line too long (83 > 79 characters)```

@ron819
Copy link
Contributor

ron819 commented Sep 5, 2018

@lorenanicole @Fokko any progress with this?

@Fokko
Copy link
Contributor

Fokko commented Sep 5, 2018

Sorry @ron819 for not responding.

@lorenanicole We started doing Flake8 on code diffs of the commits to increase the quality of the code. Therefore sometimes you have to fix these issues. So please pick them up. Do you see any possibility to rebase onto master to fix the conflict? Thanks!

@dsakuma dsakuma mentioned this pull request Oct 5, 2018
Merged
5 tasks
@ashb
Copy link
Member

ashb commented Nov 6, 2018

Merged in another Pr now.

@ashb ashb closed this Nov 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@lorenanicole @Fokko @ron819 @ashb