[AIRFLOW-4335] Add default num_retries to GCP connection #5117

ryanyuan · 2019-04-17T07:13:16Z

Add default num_retries to GCP connection

Make sure you have checked all steps below.

Jira

My PR addresses the following Airflow-4335 issues and references them in the PR title. For example, "[AIRFLOW-XXX] My Airflow PR"
- https://issues.apache.org/jira/browse/AIRFLOW-XXX
- In case you are fixing a typo in the documentation you can prepend your commit with [AIRFLOW-XXX], code changes always need a Jira issue.
- In case you are proposing a fundamental code change, you need to create an Airflow Improvement Proposal (AIP).
- In case you are adding a dependency, check if the license complies with the ASF 3rd Party License Policy.

Description

Here are some details about my PR, including screenshots of any UI changes:

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.
- All the public functions and the classes in the PR contain docstrings that explain what it does
- If you implement backwards incompatible changes, please leave a note in the Updating.md so we can assign it to a appropriate release

Code Quality

Passes flake8

codecov-io · 2019-04-17T13:02:38Z

Codecov Report

Merging #5117 into master will decrease coverage by 0.11%.
The diff coverage is 90.24%.

@@            Coverage Diff             @@
##           master    #5117      +/-   ##
==========================================
- Coverage   78.07%   77.96%   -0.12%     
==========================================
  Files         465      465              
  Lines       29784    29828      +44     
==========================================
  Hits        23254    23254              
- Misses       6530     6574      +44

Impacted Files	Coverage Δ
airflow/www/views.py	`76.38% <ø> (+0.03%)`	⬆️
airflow/contrib/hooks/gcp_pubsub_hook.py	`91.66% <100%> (+0.08%)`	⬆️
airflow/www/forms.py	`100% <100%> (ø)`	⬆️
airflow/contrib/hooks/bigquery_hook.py	`61.08% <100%> (+0.19%)`	⬆️
airflow/contrib/hooks/gcp_function_hook.py	`73.07% <100%> (ø)`	⬆️
airflow/contrib/hooks/gcp_transfer_hook.py	`95.55% <100%> (ø)`	⬆️
airflow/contrib/hooks/gcp_kms_hook.py	`93.75% <100%> (+0.2%)`	⬆️
airflow/contrib/hooks/gcp_sql_hook.py	`69.21% <100%> (ø)`	⬆️
airflow/contrib/hooks/gcp_compute_hook.py	`78.65% <100%> (ø)`	⬆️
airflow/contrib/hooks/datastore_hook.py	`100% <100%> (ø)`	⬆️
... and 22 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 dee7e67...826ebc6. Read the comment docs.

ryanyuan · 2019-04-17T23:54:29Z

This is to solve #5043

ryanyuan · 2019-04-17T23:55:46Z

@kaxil PTAL

potiuk

I like the change a lot. I think you need to squash it and rebase and we should be good to go.

potiuk · 2019-04-19T14:39:59Z

airflow/contrib/hooks/bigquery_hook.py

@@ -60,6 +60,7 @@ def __init__(self,
            gcp_conn_id=bigquery_conn_id, delegate_to=delegate_to)
        self.use_legacy_sql = use_legacy_sql
        self.location = location
+        self.num_retries = self._get_field('num_retries', 5)


I really like that you take values from parameters of Google Connection :)

If anyone is interested in this subject then the source of the idea is this comment: #5054 (comment)

potiuk · 2019-04-19T14:43:46Z

docs/howto/connection/gcp.rst

@@ -71,6 +71,12 @@ Scopes (comma separated)
        issue `AIRFLOW-2522
        <https://issues.apache.org/jira/browse/AIRFLOW-2522>`_.

+Number of Retries


Love that you remembered about documentation!

mik-laj · 2019-04-19T18:14:52Z

docs/howto/connection/gcp.rst

@@ -71,6 +71,12 @@ Scopes (comma separated)
        issue `AIRFLOW-2522
        <https://issues.apache.org/jira/browse/AIRFLOW-2522>`_.

+Number of Retries
+    Integer, number of times to retry with randomized
+    exponential backoff. If all retries fail, the HttpError


Can you use the class role to create a link to the documentation
Example:

AAA :class:`~airflow.HttpError` BBBB

Reference:
http://www.sphinx-doc.org/en/master/usage/extensions/intersphinx.html

ryanyuan · 2019-04-20T03:22:42Z

@potiuk @mik-laj Thanks for your reviews!

Commits squashed and it got rebased.

potiuk · 2019-04-20T08:35:10Z

Unfortunately there was a transient error in Travis (I hope we can get rid of those soon). Looks good, but It would be great if you reword commit message as after squash it has some duplication in.

Add default num_retries to GCP connection

ryanyuan · 2019-04-20T13:11:17Z

Unfortunately there was a transient error in Travis (I hope we can get rid of those soon). Looks good, but It would be great if you reword commit message as after squash it has some duplication in.

@potiuk Done.

OmerJog · 2019-04-21T07:03:39Z

@potiuk you'll need to close the ticket and target it to 2.0.0 :)
Do we want this for 1.10.4 ? If so it will require another PR against the V1.10 branch as it require UI changes.

potiuk · 2019-04-21T07:41:26Z

Yeah. If I only could :). Waiting for the rights to be able to do so ;)

potiuk · 2019-04-21T07:48:22Z

Also I think decision on what to merge for 1.10.4 will be done by release manager :)

Add default num_retries to GCP connection (cherry picked from commit 16e7e61)

Add default num_retries to GCP connection

Add default num_retries to GCP connection (cherry picked from commit 16e7e61)

potiuk approved these changes Apr 19, 2019

View reviewed changes

mik-laj reviewed Apr 19, 2019

View reviewed changes

ryanyuan force-pushed the gcp-conn-default-num-retries branch 3 times, most recently from c5d7cd2 to c38d6bc Compare April 20, 2019 00:54

ryanyuan force-pushed the gcp-conn-default-num-retries branch from c38d6bc to 7d26926 Compare April 20, 2019 12:27

[AIRFLOW-4335] Add default num_retries to GCP connection

826ebc6

Add default num_retries to GCP connection

ryanyuan force-pushed the gcp-conn-default-num-retries branch from 7d26926 to 826ebc6 Compare April 20, 2019 12:29

potiuk merged commit 16e7e61 into apache:master Apr 20, 2019

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

[AIRFLOW-4335] Add default num_retries to GCP connection (#5117)

9404cca

Add default num_retries to GCP connection (cherry picked from commit 16e7e61)

ashb pushed a commit to ashb/airflow that referenced this pull request Apr 29, 2019

[AIRFLOW-4335] Add default num_retries to GCP connection (apache#5117)

272e2df

Add default num_retries to GCP connection (cherry picked from commit 16e7e61)

kurtqq mentioned this pull request Jun 20, 2019

[AIRFLOW-4775] - fix incorrect parameter order in GceHook #5450

Closed

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

[AIRFLOW-4335] Add default num_retries to GCP connection (apache#5117)

5802230

Add default num_retries to GCP connection

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

[AIRFLOW-4335] Add default num_retries to GCP connection (apache#5117)

4a64c46

Add default num_retries to GCP connection

dharamsk pushed a commit to postmates/airflow that referenced this pull request Aug 8, 2019

[AIRFLOW-4335] Add default num_retries to GCP connection (apache#5117)

6661729

Add default num_retries to GCP connection (cherry picked from commit 16e7e61)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[AIRFLOW-4335] Add default num_retries to GCP connection #5117

[AIRFLOW-4335] Add default num_retries to GCP connection #5117

ryanyuan commented Apr 17, 2019 •

edited

Loading

codecov-io commented Apr 17, 2019 •

edited

Loading

ryanyuan commented Apr 17, 2019 •

edited

Loading

ryanyuan commented Apr 17, 2019

potiuk left a comment

potiuk Apr 19, 2019

mik-laj Apr 19, 2019

potiuk Apr 19, 2019

mik-laj Apr 19, 2019

ryanyuan commented Apr 20, 2019

potiuk commented Apr 20, 2019

ryanyuan commented Apr 20, 2019

OmerJog commented Apr 21, 2019 •

edited

Loading

potiuk commented Apr 21, 2019

potiuk commented Apr 21, 2019

[AIRFLOW-4335] Add default num_retries to GCP connection #5117

[AIRFLOW-4335] Add default num_retries to GCP connection #5117

Conversation

ryanyuan commented Apr 17, 2019 • edited Loading

Jira

Description

Tests

Commits

Documentation

Code Quality

codecov-io commented Apr 17, 2019 • edited Loading

Codecov Report

ryanyuan commented Apr 17, 2019 • edited Loading

ryanyuan commented Apr 17, 2019

potiuk left a comment

Choose a reason for hiding this comment

potiuk Apr 19, 2019

Choose a reason for hiding this comment

mik-laj Apr 19, 2019

Choose a reason for hiding this comment

potiuk Apr 19, 2019

Choose a reason for hiding this comment

mik-laj Apr 19, 2019

Choose a reason for hiding this comment

ryanyuan commented Apr 20, 2019

potiuk commented Apr 20, 2019

ryanyuan commented Apr 20, 2019

OmerJog commented Apr 21, 2019 • edited Loading

potiuk commented Apr 21, 2019

potiuk commented Apr 21, 2019

ryanyuan commented Apr 17, 2019 •

edited

Loading

codecov-io commented Apr 17, 2019 •

edited

Loading

ryanyuan commented Apr 17, 2019 •

edited

Loading

OmerJog commented Apr 21, 2019 •

edited

Loading