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.

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

[AIRFLOW-3346] Add hook and operator for GCP transfer service. #4189

Merged
merged 2 commits into from Nov 16, 2018
Merged

[AIRFLOW-3346] Add hook and operator for GCP transfer service. #4189

merged 2 commits into from Nov 16, 2018

Conversation

jmcarp
Copy link
Contributor

@jmcarp jmcarp commented Nov 14, 2018

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:

Add a hook and operator(s) to connect to gcp storage transfer service and transfer files from s3 to gcp (and gcp to gcp) without copying to local disk.

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

@jmcarp jmcarp force-pushed the issue-3346-gcp-transfer-service branch 5 times, most recently from d747d9d to 0dd3db8 Compare November 14, 2018 20:32
kaxil
kaxil requested changes Nov 14, 2018
View reviewed changes
Copy link
Member

@kaxil kaxil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor suggestion.

Also, you need to add this to integrations.rst and code.rst.

@potiuk Would be good to get your opinion as well.

airflow/contrib/hooks/gcp_transfer_hook.py Outdated


# noinspection PyAbstractClass
class TransferServiceHook(GoogleCloudBaseHook):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rename TransferServiceHook to GcpTransferServiceHook

airflow/contrib/operators/s3_to_gcs_transfer_operator.py Outdated
:param gcs_bucket: The destination Google Cloud Storage bucket
where you want to store the files. (templated)
:type gcs_bucket: str
:param project_id: The GCP project ID
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reword it to The ID of the Google Cloud Platform Console project that owns the job.

tests/contrib/hooks/test_gcp_transfer_hook.py Outdated
mock = None


class TestTransferServiceHook(unittest.TestCase):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TestTransferServiceHook -> TestGcpTransferServiceHook

@jmcarp jmcarp force-pushed the issue-3346-gcp-transfer-service branch 2 times, most recently from 61e5399 to 869431b Compare November 15, 2018 19:35
@codecov-io
Copy link

codecov-io commented Nov 15, 2018 •
edited

Codecov Report

Merging #4189 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff           @@
##           master   #4189   +/-   ##
======================================
  Coverage    77.7%   77.7%           
======================================
  Files         199     199           
  Lines       16312   16312           
======================================
  Hits        12675   12675           
  Misses       3637    3637

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 2b707ab...137b91d. Read the comment docs.

@jmcarp jmcarp force-pushed the issue-3346-gcp-transfer-service branch from 869431b to ad46df7 Compare November 16, 2018 19:13
@jmcarp jmcarp force-pushed the issue-3346-gcp-transfer-service branch from ad46df7 to 137b91d Compare November 16, 2018 19:16
@jmcarp
Copy link
Contributor Author

jmcarp commented Nov 16, 2018

Thanks for the update @kaxil! Let me know when you're done and I'll squash commits.

@kaxil kaxil merged commit 6b68f08 into apache:master Nov 16, 2018
@kaxil
Copy link
Member

kaxil commented Nov 16, 2018 •
edited

I have squashed and merged it. Thanks @jmcarp for your contribution :)

tmiller-msft pushed a commit to cse-airflow/incubator-airflow that referenced this pull request Nov 27, 2018
@jmcarp @tmiller-msft
[AIRFLOW-3346] Add hook and operator for GCP transfer service (apache…
d0dc171 
…#4189)
kaxil pushed a commit that referenced this pull request Dec 2, 2018
@jmcarp @kaxil
[AIRFLOW-3346] Add hook and operator for GCP transfer service (#4189)
02e294f
elizabethhalper pushed a commit to cse-airflow/incubator-airflow that referenced this pull request Dec 7, 2018
@jmcarp @elizabethhalper
[AIRFLOW-3346] Add hook and operator for GCP transfer service (apache…
2c8bd4d 
…#4189)
aliceabe pushed a commit to aliceabe/incubator-airflow that referenced this pull request Jan 3, 2019
@jmcarp
[AIRFLOW-3346] Add hook and operator for GCP transfer service (apache…
4855ec6 
…#4189)
kaxil pushed a commit that referenced this pull request Jan 9, 2019
@jmcarp @kaxil
[AIRFLOW-3346] Add hook and operator for GCP transfer service (#4189)
d506897
ashb pushed a commit to ashb/airflow that referenced this pull request Jan 10, 2019
@jmcarp @ashb
[AIRFLOW-3346] Add hook and operator for GCP transfer service (apache…
d32e9e1 
…#4189)
cfei18 pushed a commit to cfei18/incubator-airflow that referenced this pull request Jan 23, 2019
@jmcarp
Add hook and operator for GCP transfer service (apache#4189)
fd8ead7
wmorris75 pushed a commit to modmed/incubator-airflow that referenced this pull request Jul 29, 2019
@jmcarp
[AIRFLOW-3346] Add hook and operator for GCP transfer service (apache…
6b3e2f6 
…#4189)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kaxil kaxil kaxil approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@jmcarp @codecov-io @kaxil