chore(bigquery-jdbc): enable proxy tests#13617
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the integration tests to reuse a shared BigQuery client instance initialized from the JDBC connection, eliminating redundant instantiations of the BigQuery service across multiple test classes. Additionally, ITDriverTest is updated to extend ITBase to leverage shared connection configurations. Feedback is provided regarding a potential issue in ITStatementTest.java where a double semicolon is introduced in the connection URL string concatenation.
…c/it/ITStatementTest.java Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
| Connection connectionUS = | ||
| DriverManager.getConnection( | ||
| String.format(CONNECTION_URL, DEFAULT_CATALOG, OAUTH_TYPE, "us-east5")); | ||
| Connection connectionUS = DriverManager.getConnection(CONNECTION_URL); |
There was a problem hiding this comment.
nit: use try-with resource
| protected static BigQuery getBigQuery(String connectionUrl) { | ||
| try { | ||
| return DriverManager.getConnection(connectionUrl) | ||
| .unwrap(BigQueryConnection.class) | ||
| .getBigQuery(); | ||
| } catch (SQLException e) { | ||
| throw new RuntimeException("Failed to initialize BigQuery client", e); | ||
| } | ||
| } |
There was a problem hiding this comment.
Doubt: So here we are unwrapping the BigQuery client from the connection directly. Since we don't save a reference to the Connection itself, how will it get closed later in the tests? I'm just thinking about potential resource leaks or background threads staying open
2 participants
Replace all places where BigQuery client is instantiated with the one instantiated via the Driver. It allows to use the same connection properties and run tests in various environments without too much of duplication for default location, endpoint overrides, proxy, TPC etc.
I ran
make docker-proxy-integration-testfor Presubmit tests and all of them passed (except for KMS which I don't have local config for).In the next PR I plan adding proxy tests runs to nightly tests & working on EU-location tests next.