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

docs: add sample for timeout for one RPC #707

Merged
merged 6 commits into from Jan 6, 2021
Merged

Conversation

olavloite
Copy link
Contributor

@olavloite olavloite commented Dec 10, 2020

Adds a sample for setting a timeout for a single RPC.

Fixes GoogleCloudPlatform/java-docs-samples#3805

Adds a sample for setting a timeout for a single RPC.

Fixes GoogleCloudPlatform/java-docs-samples#3805
@olavloite olavloite requested a review from thiagotnunes Dec 10, 2020
@olavloite olavloite requested a review from as a code owner Dec 10, 2020
@olavloite olavloite requested a review from Dec 10, 2020
@google-cla google-cla bot added the cla: yes label Dec 10, 2020
@product-auto-label product-auto-label bot added api: spanner samples labels Dec 10, 2020
@olavloite olavloite added the kokoro:force-run label Dec 10, 2020
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run label Dec 10, 2020
@eaball35
Copy link
Contributor

@eaball35 eaball35 commented Dec 10, 2020

Screen Shot 2020-12-10 at 9 00 28 AM

@olavloite
Copy link
Contributor Author

@olavloite olavloite commented Dec 10, 2020

@eaball35 These RESOURCE_EXHAUSTED errors have been mitigated by #669, which is included in version 3.1.0. The sample builds are currently using version 3.0.4. I'm afraid the only possible mitigation at this moment is retrying the test run.

@olavloite olavloite added the kokoro:force-run label Dec 10, 2020
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run label Dec 10, 2020
@eaball35 eaball35 added the kokoro:force-run label Dec 10, 2020
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run label Dec 10, 2020
@olavloite olavloite added the kokoro:force-run label Dec 10, 2020
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run label Dec 10, 2020
Copy link
Contributor

@lesv lesv left a comment

Testing can't possibly ever work.

@olavloite olavloite added the kokoro:force-run label Dec 11, 2020
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run label Dec 11, 2020
@olavloite olavloite added the kokoro:force-run label Dec 11, 2020
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run label Dec 11, 2020
@codecov
Copy link

@codecov codecov bot commented Dec 14, 2020

Codecov Report

Merging #707 (e6a34d9) into master (d49517f) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #707   +/-   ##
=========================================
  Coverage     85.05%   85.05%           
  Complexity     2556     2556           
=========================================
  Files           142      142           
  Lines         13930    13930           
  Branches       1326     1326           
=========================================
  Hits          11848    11848           
  Misses         1526     1526           
  Partials        556      556           

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 d49517f...e6a34d9. Read the comment docs.

@olavloite olavloite requested a review from lesv Dec 14, 2020
@olavloite
Copy link
Contributor Author

@olavloite olavloite commented Dec 16, 2020

@lesv Would you mind taking a second look at this PR? The PR is currently blocked from merging as you requested changes.

@lesv
Copy link
Contributor

@lesv lesv commented Jan 5, 2021

@olavloite Apologies for the delay, what I'm concerned about is that we might test with many PR's / java version tests running at the same time, I'm concerned that you will have collisons.

I was probably stuck on table names of Singers & Venues, but that's not really the problem since you are creating an instance and then deleting it. The code in createTestDatabase appears to use EnvVar, it's unclear if that's unique. It also falls back to using the last instanceId, but that looks like it should at least have a while in there somewhere, and actually go to a next id rather than the last one to have something unique.

I'm going to release my objection so you can move forward - again I missed the email or I'd have responded earlier. Apologies.

If you believe I'm wrong in my concern, feel free to merge, if not could you fix it and explain where my understanding is flawed?

lesv
lesv approved these changes Jan 6, 2021
@lesv
Copy link
Contributor

@lesv lesv commented Jan 6, 2021

I found the discussion. LGTM

@thiagotnunes thiagotnunes merged commit 056f54f into master Jan 6, 2021
21 checks passed
@thiagotnunes thiagotnunes deleted the add-sample-for-timeout branch Jan 6, 2021
thiagotnunes pushed a commit that referenced this issue May 6, 2021
* docs: add sample for timeout for one RPC

Adds a sample for setting a timeout for a single RPC.

Fixes GoogleCloudPlatform/java-docs-samples#3805

* fix: fix import order

* fix: format code with the correct formatter plugin

* fix: delete test data after each test

* fix: auto-throttle admin requests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: spanner cla: yes samples
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants