Skip to content

fix(bigquery-jdbc): configure logging early to capture URL parsing warnings and errors#13323

Merged
Neenu1995 merged 8 commits into
mainfrom
log-startup-errors
Jun 2, 2026
Merged

fix(bigquery-jdbc): configure logging early to capture URL parsing warnings and errors#13323
Neenu1995 merged 8 commits into
mainfrom
log-startup-errors

Conversation

@Neenu1995
Copy link
Copy Markdown
Contributor

No description provided.

@Neenu1995 Neenu1995 requested review from a team as code owners June 1, 2026 20:14
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 refactors connection URL parsing in BigQueryDriver to extract logging configuration properties (LogLevel and LogPath) before performing full validation. This ensures that the logger is initialized early enough to capture any parsing errors or warnings, which are now covered by new unit tests. Feedback on the changes highlights a potential bug in BigQueryJdbcUrlUtility.parseUriPropertyWithoutValidation where converting the URI to uppercase to find indices and then applying them to the original URI can cause StringIndexOutOfBoundsException in certain locales. A safer, split-based parsing approach is suggested.

@Neenu1995
Copy link
Copy Markdown
Contributor Author

/gemini review

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 refactors the BigQuery JDBC driver to extract logging configuration properties from the connection URL before initializing the DataSource, enabling logging to start earlier. It also adds unit tests to verify that warnings and errors are logged correctly. The review feedback highlights a compilation error in BigQueryDriver due to an invalid Logger method call, and suggests using AssertJ's assertThatThrownBy in the new tests for cleaner and more idiomatic exception assertions.

@Neenu1995 Neenu1995 enabled auto-merge (squash) June 2, 2026 14:45
@Neenu1995 Neenu1995 merged commit 98d8e69 into main Jun 2, 2026
176 of 177 checks passed
@Neenu1995 Neenu1995 deleted the log-startup-errors branch June 2, 2026 15:07
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.

3 participants