Skip to content

FEAT: rewire bigtable.internal.sessions-required system property - #14078

Closed
neilw4 wants to merge 1 commit into
googleapis:mainfrom
neilw4:forcejs
Closed

FEAT: rewire bigtable.internal.sessions-required system property#14078
neilw4 wants to merge 1 commit into
googleapis:mainfrom
neilw4:forcejs

Conversation

@neilw4

@neilw4 neilw4 commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

This is used internally for testing

@neilw4
neilw4 requested review from a team as code owners August 14, 2026 12:40

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

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.

Code Review

This pull request introduces support for the "bigtable.internal.sessions-required" system property in ClientConfigurationManager.java. When enabled, it sets a default session load of 1.0f if the configured session load is zero, and ensures areSessionsRequired() returns true. The feedback suggests defining a constant for the duplicated system property key to improve maintainability.

if (builder.getSessionConfiguration().getSessionLoad() == 0) {
builder.clearSessionConfiguration();
return builder.build();
if (Boolean.getBoolean("bigtable.internal.sessions-required")) {

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 system property key "bigtable.internal.sessions-required" is duplicated here and on line 425. Consider defining a constant for the property name to improve maintainability. Note that caching system properties or environment variables in class fields is unnecessary if they are only evaluated once during object initialization.

References
  1. Avoid caching system properties or environment variables in class fields if they are only evaluated once during object initialization, as caching is unnecessary in such cases.

@mutianf mutianf closed this Aug 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants