Skip to content

[AIRFLOW-2825]Fix S3ToHiveTransfer bug due to case#3665

Merged
Fokko merged 1 commit intoapache:masterfrom
XD-DENG:patch-6
Jul 31, 2018
Merged

[AIRFLOW-2825]Fix S3ToHiveTransfer bug due to case#3665
Fokko merged 1 commit intoapache:masterfrom
XD-DENG:patch-6

Conversation

@XD-DENG
Copy link
Copy Markdown
Member

@XD-DENG XD-DENG commented Jul 30, 2018

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:

Because upper/lower case was not considered in the file extension check, S3ToHiveTransfer operator may mistakenly think a GZIP file with uppercase ext ".GZ" (or ".gZ", ".Gz") is not a GZIP file and raise exception.

Tests

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

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

@codecov-io
Copy link
Copy Markdown

codecov-io commented Jul 30, 2018

Codecov Report

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

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3665      +/-   ##
==========================================
- Coverage   77.51%   77.51%   -0.01%     
==========================================
  Files         205      205              
  Lines       15751    15751              
==========================================
- Hits        12210    12209       -1     
- Misses       3541     3542       +1
Impacted Files Coverage Δ
airflow/operators/s3_to_hive_operator.py 93.96% <ø> (ø) ⬆️
airflow/models.py 88.54% <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 dfa7b26...c7e5446. Read the comment docs.

@feng-tao
Copy link
Copy Markdown
Member

could you add a test?

@XD-DENG XD-DENG force-pushed the patch-6 branch 2 times, most recently from 50e9dff to 4413ea6 Compare July 31, 2018 01:31
Because upper/lower case was not considered
in the file extension check, S3ToHiveTransfer
operator may mistakenly think a GZIP file with
uppercase ext ".GZ" is not a GZIP file and
raise exception.
@XD-DENG
Copy link
Copy Markdown
Member Author

XD-DENG commented Jul 31, 2018

Hi @feng-tao, thanks for suggesting this.

I have updated the related test. Instead of adding separate testing items, I updated the existing ones.

@Fokko
Copy link
Copy Markdown
Contributor

Fokko commented Jul 31, 2018

LGTM, thanks @XD-DENG

@Fokko Fokko merged commit 8d2f57c into apache:master Jul 31, 2018
@XD-DENG XD-DENG deleted the patch-6 branch August 1, 2018 02:52
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