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-1338] Fix incompatible GCP dataflow hook #2388

Closed
wants to merge 1 commit into from

Conversation

fenglu-g
Copy link
Contributor

GCP dataflow hook is incompatible with recent google-cloud-dataflow
/apache-beam release (>=2.0.0). The DataflowPipelineRunner has
renamed to DataflowRunner, which breaks the existing
gcp_dataflow_hook. This PR updates the runner name.

FYI- this is the apache beam commit (https://goo.gl/9PhGQ6) that
introduces the incompatible change.

@codecov-io
Copy link

Codecov Report

Merging #2388 into master will increase coverage by 0.02%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2388      +/-   ##
==========================================
+ Coverage   69.23%   69.26%   +0.02%     
==========================================
  Files         146      146              
  Lines       11227    11227              
==========================================
+ Hits         7773     7776       +3     
+ Misses       3454     3451       -3
Impacted Files Coverage Δ
airflow/models.py 87.12% <0%> (-0.05%) ⬇️
airflow/jobs.py 75.27% <0%> (+0.4%) ⬆️

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 9958aa9...775e84c. Read the comment docs.

@criccomini
Copy link
Contributor

@alexvanboxel want to have a look?

@criccomini
Copy link
Contributor

+1 merged.. I think @alexvanboxel is out, and changes LGTM

@asfgit asfgit closed this in cf2605d Jun 23, 2017
@fenglu-g
Copy link
Contributor Author

Thanks @criccomini, I'll sync with @yk5 to fix the updating.md.

@@ -142,6 +142,7 @@ def check_previous():
'google-api-python-client>=1.5.0, <1.6.0',
'oauth2client>=2.0.2, <2.1.0',
'PyOpenSSL',
'google-cloud-dataflow',
Copy link
Member

Choose a reason for hiding this comment

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

@fenglu-g @Fokko Do we really need the 'google-cloud-dataflow' library here as what I can see is we are using Google's discovery based API for all google cloud related commands?

Please correct me if I am wrong or if I misinterpreted something.

Copy link
Member

Choose a reason for hiding this comment

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

The reason I am asking this is due to the issues with dependencies as stated in this 2 Jira issues:

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see, fine with removing this for now. We'll fix it on the google-data-flow side.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks @fenglu-g . I shall submit a PR removing 'google-cloud-dataflow' and will mention you so that you can approve it.

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.

None yet

4 participants