X Tutup
The Wayback Machine - https://web.archive.org/web/20201220073746/https://github.com/googleapis/nodejs-bigquery/pull/864
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

fix: do not retry jobs.insert when it flakes #864

Merged
merged 5 commits into from Nov 2, 2020

Conversation

@stephenplusplus
Copy link
Member

@stephenplusplus stephenplusplus commented Sep 28, 2020

Fixes #857

This is an attempt to handle a situation where we need to create an ID for a job. Before this PR:

  • createJob() is called
  • we generate a UUID and create a job
  • the API processes the request and creates a job in the background
  • the API call flakes and returns an error
  • our auto-retry logic attempts the request again
  • the API call falls with a 409: Already Exists error
  • we return the 409 to the user

This PR will make it so that when the request is "auto-retried" and returns the 409, we'll ignore it, and instead refresh the metadata of the Job, treating that as if it was the original response from the create request.

  • createJob() is called
  • we generate a UUID and create a job
  • the API processes the request and creates a job in the background
  • the API call flakes and returns an error
  • our auto-retry logic attempts the request again
  • the API call falls with a 409: Already Exists error
  • we return the 409 to the user refresh the metadata from the Job and return it to the user

To Dos

  • Unit tests
@steffnay
Copy link
Contributor

@steffnay steffnay commented Oct 1, 2020

@stephenplusplus this looks good to me so far

@stephenplusplus stephenplusplus force-pushed the stephenplusplus:spp--857 branch from 899c2d0 to 6144638 Oct 16, 2020
@codecov
Copy link

@codecov codecov bot commented Oct 16, 2020

Codecov Report

Merging #864 into master will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #864   +/-   ##
=======================================
  Coverage   98.65%   98.65%           
=======================================
  Files           9        9           
  Lines        6986     7003   +17     
  Branches      422      471   +49     
=======================================
+ Hits         6892     6909   +17     
  Misses         94       94           
Impacted Files Coverage Δ
src/bigquery.ts 99.95% <100.00%> (+<0.01%) ⬆️

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 96f939c...08007ae. Read the comment docs.

@stephenplusplus stephenplusplus requested a review from steffnay Oct 28, 2020
steffnay and others added 2 commits Nov 2, 2020
@stephenplusplus stephenplusplus merged commit 255491b into googleapis:master Nov 2, 2020
14 checks passed
14 checks passed
test (10)
Details
test (12)
Details
test (14)
Details
test (15)
Details
windows
Details
lint lint
Details
docs
Details
ci/kokoro: Samples test Build successful
Details
ci/kokoro: System test Build successful
Details
cla/google All necessary CLAs are signed
codecov/patch 100.00% of diff hit (target 98.65%)
Details
codecov/project 98.65% (+0.00%) compared to 96f939c
Details
conventionalcommits.org
Details
header-check Headercheck
Details
@stephenplusplus stephenplusplus deleted the stephenplusplus:spp--857 branch Nov 2, 2020
@BenBirt
Copy link
Contributor

@BenBirt BenBirt commented Nov 2, 2020

Thanks for implementing this @stephenplusplus !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

3 participants
You can’t perform that action at this time.
X Tutup