[AIRFLOW-3971] Add Google Cloud Natural Language operators by mik-laj · Pull Request #4980 · apache/airflow

mik-laj · 2019-03-26T12:04:44Z

Make sure you have checked all steps below.

Jira

https://issues.apache.org/jira/browse/AIRFLOW-3971

Description

Hello,

I would like to create operators for Google Natural Language:

CloudLanguageAnalyzeEntitiesOperator
CloudLanguageAnalyzeEntitySentimentOperator
CloudLanguageAnalyzeSentimentOperator
CloudLanguageClassifyTextOperator

Tests

Unit tests for hook
Unit tests for operators
System test

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

How-to Guide
Integratio.rst
API Reference

Code Quality

Passes flake8

wwlian · 2019-03-29T17:54:11Z

Please use the full name of the API: Google Cloud Natural Language API.

I will do it.

Do you think it's worth updating the google-cloud-python library to use the correct name?
https://github.com/googleapis/google-cloud-python/blob/master/docs/index.rst

Good point, yes, I think that would make sense.

wwlian · 2019-03-29T18:01:04Z

I'm not sure if this is just a rendering problem during review, but all of these autoclass:: entries don't seem to be rendering correctly for me here:
https://github.com/apache/airflow/blob/b66ea5b2a1dd87572e2ac5626fce9ea164bd8097/docs/code.rst

They just show up as text literals in a code-style text box.

Github does not support additional directives provided by Sphinx. Ever since I made this PR, the format of the documentation has also changed.

Reference:
https://lists.apache.org/thread.html/7c4a303a9de81c9b49de2f47589c799db12ad4bcfd66530309a09dc1@%3Cdev.airflow.apache.org%3E

mik-laj · 2019-03-29T18:18:32Z

@wwlian If you are interested, I will prepare you preview of the built-in documentation in HTML format.

mik-laj · 2019-03-29T18:34:06Z

Preview: http://classy-fan.surge.sh/howto/operator/gcp/natural_language.html

mik-laj · 2019-03-29T19:08:24Z

PTAL @kaxil I considered all the suggestions of the community.

kaxil · 2019-03-31T23:08:44Z

Suggested change

Example Airflow DAG for Google Natural Language service

Example Airflow DAG for Google Cloud Natural Language service

kaxil · 2019-03-31T23:10:44Z

Can we add gcp here... as other cloud providers might have similar services and can conflict in future?

Suggested change

# [START howto_operator_natural_language_analyze_entities]

# [START howto_gcp_operator_natural_language_analyze_entities]

And same in all the comments below

Conflict will never happen because the tags are always associated with the file. Prefixes are completely unnecessary. However, I will introduce your suggestion in a moment.

aah.. yes you right. :-) Probably I shouldn't be reviewing so late... brain-fade moment 😄

I prefer to use 'howto_operator_gcp_natural_language_analyze_entities` to mark the file that contains these sections. I did not add the word "GCP" earlier because I thought it was a sufficient level of depth to easily find a file.

yea agree.. Can you resolve conflicts with docs/conf.py

Yes. I'm working on it now.

mik-laj · 2019-03-31T23:40:01Z

@kaxil I apply all your suggestion. In addition, I changed all literalinclude to exampleinclude, so full code of example is available now. :-)
Reference:
#4894

kaxil · 2019-04-01T19:45:30Z

Suggested change

pip install airflow[gcp]

pip install apache-airflow[gcp]

Thanks. I updated a PR.

kaxil · 2019-04-01T19:51:23Z

Can we update this to httplib2~=0.9.2, so that the major version update doesn't break Airflow.

Yes. I will do it.

mik-laj · 2019-04-01T22:12:08Z

@kaxil Do you have any comments about the documentation? In this PR I introduce a new structure of documentation, which will be a lot user-friendly. It contains more information, and information that already exists is presented in a different form. I would like the documentation to be understandable for a novice user. The previous form did not meet my expectations. While creating this documentation, I tried to enter all the suggestions that were reported to the previous documentation from my client. I will be grateful for your comments and opinions..

Preview of latest version is available: http://two-pain.surge.sh/howto/operator/gcp/natural_language.html

kaxil

LGTM. And the documentation looks quite detailed as well. Well done @mik-laj :)

kaxil · 2019-04-04T09:16:19Z

I don't know why CI has not been triggered for this PR. 🤔

mik-laj · 2019-04-04T09:39:03Z

@kaxil I do not understand it either. I can run the task on my forks if it helps you.

mik-laj · 2019-04-04T09:46:12Z

A trigger a new build on my Travis. Status is available: PolideaInternal#74

kaxil · 2019-04-04T23:25:34Z

the CI is failing @mik-laj

mik-laj · 2019-04-04T23:29:35Z

@kaxil Thanks. I working on it now.

codecov-io · 2019-04-05T00:57:11Z

Codecov Report

Merging #4980 into master will increase coverage by <.01%.
The diff coverage is 78.22%.

@@            Coverage Diff             @@
##           master    #4980      +/-   ##
==========================================
+ Coverage   76.23%   76.24%   +<.01%     
==========================================
  Files         466      469       +3     
  Lines       30101    30225     +124     
==========================================
+ Hits        22949    23045      +96     
- Misses       7152     7180      +28

Impacted Files	Coverage Δ
airflow/contrib/hooks/gcp_api_base_hook.py	`84.76% <ø> (ø)`	⬆️
...ntrib/example_dags/example_gcp_natural_language.py	`0% <0%> (ø)`
...contrib/operators/gcp_natural_language_operator.py	`100% <100%> (ø)`
airflow/contrib/hooks/gcp_natural_language_hook.py	`91.17% <91.17%> (ø)`
airflow/models/__init__.py	`92.95% <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 463c183...d04c780. Read the comment docs.

mik-laj · 2019-04-05T09:19:18Z

@kaxil it's green now. I replace a method import_module with a method from autodoc sphinx extension (it's part of core) and add a mock support based on autodoc code.

kaxil

Awesome, LGTM

mik-laj force-pushed the feature/gcp-natural-lanaguage-operators branch 2 times, most recently from 3873ac6 to b66ea5b Compare March 29, 2019 14:03

wwlian reviewed Mar 29, 2019

View reviewed changes

mik-laj force-pushed the feature/gcp-natural-lanaguage-operators branch 2 times, most recently from 979aa0c to 49b98ab Compare March 29, 2019 19:07

mik-laj mentioned this pull request Mar 30, 2019

Update Natural Language service name googleapis/google-cloud-python#7624

Merged

kaxil requested changes Mar 31, 2019

View reviewed changes

mik-laj force-pushed the feature/gcp-natural-lanaguage-operators branch from 49b98ab to 7d4f7ca Compare March 31, 2019 23:37

kaxil requested changes Apr 1, 2019

View reviewed changes

mik-laj force-pushed the feature/gcp-natural-lanaguage-operators branch from 7d4f7ca to 447de8a Compare April 1, 2019 22:12

kaxil approved these changes Apr 4, 2019

View reviewed changes

mik-laj force-pushed the feature/gcp-natural-lanaguage-operators branch from 447de8a to 4868ad6 Compare April 4, 2019 09:45

mik-laj force-pushed the feature/gcp-natural-lanaguage-operators branch 6 times, most recently from 37db3a8 to 0e1f0d6 Compare April 4, 2019 21:26

[AIRFLOW-3971] Add Google Cloud Natural Language operators

d04c780

mik-laj force-pushed the feature/gcp-natural-lanaguage-operators branch from 0e1f0d6 to 81b9ce2 Compare April 5, 2019 00:10

mik-laj force-pushed the feature/gcp-natural-lanaguage-operators branch from 81b9ce2 to d04c780 Compare April 5, 2019 09:55

kaxil approved these changes Apr 5, 2019

View reviewed changes

kaxil merged commit 99c8a6f into apache:master Apr 5, 2019

kaxil pushed a commit that referenced this pull request Apr 5, 2019

[AIRFLOW-3971] Add Google Cloud Natural Language operators (#4980)

b789afd

ashb pushed a commit that referenced this pull request Apr 5, 2019

[AIRFLOW-3971] Add Google Cloud Natural Language operators (#4980)

91b856f

cthenderson pushed a commit to cthenderson/apache-airflow that referenced this pull request Apr 16, 2019

[AIRFLOW-3971] Add Google Cloud Natural Language operators (apache#4980)

e64bcee

andriisoldatenko pushed a commit to andriisoldatenko/airflow that referenced this pull request Jul 26, 2019

[AIRFLOW-3971] Add Google Cloud Natural Language operators (apache#4980)

247d21c

wmorris75 pushed a commit to modmed-external/incubator-airflow that referenced this pull request Jul 29, 2019

[AIRFLOW-3971] Add Google Cloud Natural Language operators (apache#4980)

46d5aa9

	Example Airflow DAG for Google Natural Language service
	Example Airflow DAG for Google Cloud Natural Language service

	# [START howto_operator_natural_language_analyze_entities]
	# [START howto_gcp_operator_natural_language_analyze_entities]

Conversation

mik-laj commented Mar 26, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Jira

Description

Tests

Commits

Documentation

Code Quality

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mik-laj commented Mar 29, 2019

Uh oh!

mik-laj commented Mar 29, 2019

Uh oh!

mik-laj commented Mar 29, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mik-laj commented Mar 31, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mik-laj commented Apr 1, 2019

Uh oh!

kaxil left a comment

Choose a reason for hiding this comment

Uh oh!

kaxil commented Apr 4, 2019

Uh oh!

mik-laj commented Apr 4, 2019

Uh oh!

mik-laj commented Apr 4, 2019

Uh oh!

kaxil commented Apr 4, 2019

Uh oh!

mik-laj commented Apr 4, 2019

Uh oh!

codecov-io commented Apr 5, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

mik-laj commented Apr 5, 2019

mik-laj commented Mar 26, 2019 •

edited

Loading

mik-laj commented Mar 31, 2019 •

edited

Loading

codecov-io commented Apr 5, 2019 •

edited

Loading