Skip to content

feat(spanner): Enable EEF for cloud spanner when direct access is enabled#14414

Open
kinsaurralde wants to merge 2 commits intogoogleapis:mainfrom
kinsaurralde:main
Open

feat(spanner): Enable EEF for cloud spanner when direct access is enabled#14414
kinsaurralde wants to merge 2 commits intogoogleapis:mainfrom
kinsaurralde:main

Conversation

@kinsaurralde
Copy link
Copy Markdown
Contributor

No description provided.

@kinsaurralde kinsaurralde requested review from a team as code owners April 9, 2026 23:52
@kinsaurralde kinsaurralde changed the title Enable EEF for cloud spanner when direct access is enabled feat(spanner): Enable EEF for cloud spanner when direct access is enabled Apr 9, 2026
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request updates the Spanner client to enable GCP fallback by default and configures a fallback period of three minutes. Feedback was provided to improve the robustness of environment variable parsing, ensuring that invalid or malformed values do not accidentally disable the fallback feature.

Comment thread spanner/client.go Outdated
Comment on lines +531 to +534
isFallbackEnabled := true // Default behavior when unset
if val, ok := os.LookupEnv("GOOGLE_SPANNER_ENABLE_GCP_FALLBACK"); ok {
isFallbackEnabled, _ = strconv.ParseBool(val)
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The current implementation of environment variable parsing for GOOGLE_SPANNER_ENABLE_GCP_FALLBACK will set isFallbackEnabled to false if the environment variable is set to an invalid boolean string (e.g., a typo). Since the intended default behavior is true when the variable is unset, it is more robust to preserve that default if a malformed value is provided. This ensures that the feature remains enabled unless explicitly disabled with a valid 'false' value, adhering to the principle of not altering defaults of existing stable code paths.

isFallbackEnabled := true
if val, ok := os.LookupEnv("GOOGLE_SPANNER_ENABLE_GCP_FALLBACK"); ok {
	if b, err := strconv.ParseBool(val); err == nil {
		isFallbackEnabled = b
	}
}
References
  1. When introducing new components with different default behaviors, avoid altering the defaults of existing, stable code paths to prevent breaking changes for backward compatibility.

@product-auto-label product-auto-label Bot added the api: spanner Issues related to the Spanner API. label Apr 10, 2026
Comment thread spanner/client.go
fbOpts.EnableFallback = true
fbOpts.ErrorRateThreshold = 1
fbOpts.MinFailedCalls = 1
fbOpts.Period = time.Minute * 3
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is there any issue/discussion for deciding 3 minute specifically?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api: spanner Issues related to the Spanner API.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants