Skip to content

[AIRFLOW-4782] Make GCP hooks Pylint compatible#5431

Merged
potiuk merged 1 commit intoapache:masterfrom
PolideaInternal:pylint-gcp
Jun 24, 2019
Merged

[AIRFLOW-4782] Make GCP hooks Pylint compatible#5431
potiuk merged 1 commit intoapache:masterfrom
PolideaInternal:pylint-gcp

Conversation

@turbaszek
Copy link
Member

@turbaszek turbaszek commented Jun 19, 2019

Make sure you have checked all steps below.

Jira

Description

  • This PR makes GCP hooks Pylint compatible.

Code Quality

  • Passes flake8

"""
Interact with Google Cloud KMS. This hook uses the Google Cloud Platform
connection.

Copy link
Member

Choose a reason for hiding this comment

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

It seems to me that this is not true, because no method in this class needs project_id

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for that!

class PubSubException(Exception):
pass
# Alias for Exception
PubSubException = Exception
Copy link
Member

Choose a reason for hiding this comment

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

It seems to me that it changes the behavior of the code. What error is it related to?

Copy link
Member Author

Choose a reason for hiding this comment

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

Unnecessary pass statement but this could be solved by leaving a docstring instead of bare pass.

@cached_property
def annotator_client(self):
"""
Creates ImageAnnotatorClient.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Creates ImageAnnotatorClient.
Creates ImageAnnotatorClient.

Empty line is required after description.

@codecov-io
Copy link

codecov-io commented Jun 24, 2019

Codecov Report

❗ No coverage uploaded for pull request base (master@05c1ba0). Click here to learn what that means.
The diff coverage is 73.77%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #5431   +/-   ##
=========================================
  Coverage          ?   79.12%           
=========================================
  Files             ?      488           
  Lines             ?    30665           
  Branches          ?        0           
=========================================
  Hits              ?    24263           
  Misses            ?     6402           
  Partials          ?        0
Impacted Files Coverage Δ
airflow/contrib/sensors/gcp_transfer_sensor.py 100% <ø> (ø)
airflow/contrib/hooks/gcp_pubsub_hook.py 91.66% <100%> (ø)
airflow/contrib/hooks/gcp_translate_hook.py 71.42% <100%> (ø)
airflow/contrib/hooks/gcp_container_hook.py 100% <100%> (ø)
airflow/contrib/hooks/gcp_speech_to_text_hook.py 81.25% <100%> (ø)
airflow/contrib/hooks/gcp_kms_hook.py 93.93% <100%> (ø)
airflow/contrib/operators/gcp_transfer_operator.py 95.57% <100%> (ø)
airflow/contrib/hooks/gcp_api_base_hook.py 84.9% <100%> (ø)
airflow/contrib/hooks/gcp_text_to_speech_hook.py 80% <100%> (ø)
airflow/contrib/hooks/gcp_bigtable_hook.py 93.33% <100%> (ø)
... and 12 more

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 05c1ba0...72132c2. Read the comment docs.

Copy link
Member

@potiuk potiuk left a comment

Choose a reason for hiding this comment

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

Please take a look if we can get rid of the "no-member" by annotating get_conn() return type

@potiuk potiuk merged commit 7d90446 into apache:master Jun 24, 2019
andriisoldatenko pushed a commit to andriisoldatenko/airflow that referenced this pull request Jul 26, 2019
wmorris75 pushed a commit to modmed-external/incubator-airflow that referenced this pull request Jul 29, 2019
@turbaszek turbaszek deleted the pylint-gcp branch September 19, 2019 11:54
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