Skip to content
This repository was archived by the owner on Dec 14, 2025. It is now read-only.

feat: add environment variable for configuring environment detection#275

Merged
bcoe merged 2 commits intomasterfrom
configurable-is-available-retries
Dec 16, 2019
Merged

feat: add environment variable for configuring environment detection#275
bcoe merged 2 commits intomasterfrom
configurable-is-available-retries

Conversation

@bcoe
Copy link
Copy Markdown

@bcoe bcoe commented Dec 13, 2019

This introduces a configuration variable that allows the method used to detect GCP environments, DETECT_GCP_RETRIES, to be configured.

This is based on the hypothesis that both IP and DNS environment detection are sometimes failing.

CC: @feywind, I would love to see if this addresses the issues you were seeing during the bootstrapping of @google-cloud/pubsub.

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Dec 13, 2019
@bcoe
Copy link
Copy Markdown
Author

bcoe commented Dec 13, 2019

@codecov
Copy link
Copy Markdown

codecov bot commented Dec 13, 2019

Codecov Report

Merging #275 into master will increase coverage by 0.69%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #275      +/-   ##
==========================================
+ Coverage   89.93%   90.62%   +0.69%     
==========================================
  Files           1        1              
  Lines         149      160      +11     
  Branches       35       37       +2     
==========================================
+ Hits          134      145      +11     
  Misses         14       14              
  Partials        1        1
Impacted Files Coverage Δ
src/index.ts 90.62% <100%> (+0.69%) ⬆️

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 c5767c9...97350b2. Read the comment docs.

*/
function detectGCPAvailableRetries(): number {
return process.env.DETECT_GCP_RETRIES
? Number(process.env.DETECT_GCP_RETRIES)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

It's ok if this ends up as a NaN right? Seems like it might do the same thing as zero anyway

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I think this edge-case is okay, I mostly want this option here to allow for some directed experiments with customers. If we find that a certain number of retries works better, we might remove this configuration and try to bake retries back into the client.

@bcoe bcoe merged commit 580cfa4 into master Dec 16, 2019
@bcoe bcoe deleted the configurable-is-available-retries branch December 16, 2019 14:16
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

cla: yes This human has signed the Contributor License Agreement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants