Skip to content

Issue#38 Add targeted MySQL↔BigQuery CI coverage#43

Open
kantselovich wants to merge 9 commits into
masterfrom
bq_issue_38
Open

Issue#38 Add targeted MySQL↔BigQuery CI coverage#43
kantselovich wants to merge 9 commits into
masterfrom
bq_issue_38

Conversation

@kantselovich
Copy link
Copy Markdown

@kantselovich kantselovich commented Apr 27, 2026

Fixes #38

Copy link
Copy Markdown

@orca-security-us orca-security-us Bot left a comment

Choose a reason for hiding this comment

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

Orca Security Scan Summary

Status Check Issues by priority
Passed Passed Infrastructure as Code high 0   medium 0   low 0   info 0 View in Orca
Passed Passed SAST high 0   medium 0   low 0   info 0 View in Orca
Passed Passed Secrets high 0   medium 0   low 0   info 0 View in Orca
Passed Passed Vulnerabilities high 0   medium 0   low 0   info 0 View in Orca

Copy link
Copy Markdown

@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 BigQuery database connector to automatically configure a default dataset in the query job configuration and adds BigQuery-related environment variables to the development environment file. Feedback includes a recommendation to safely handle the query job configuration to avoid potential TypeErrors when overlapping with keyword arguments, and a suggestion to use placeholders in the environment file instead of hardcoded local paths and project IDs to improve portability.

Comment on lines +255 to +257
default_config = bigquery.QueryJobConfig(default_dataset=f"{project}.{dataset}")

self._client = bigquery.Client(project=project, credentials=credentials, default_query_job_config=default_config, **kw)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

Passing default_query_job_config as an explicit keyword argument while it might also be present in **kw will cause a TypeError (multiple values for argument) if a user provides it in the connection parameters. It is safer to pop it from kw and then configure it.

Suggested change
default_config = bigquery.QueryJobConfig(default_dataset=f"{project}.{dataset}")
self._client = bigquery.Client(project=project, credentials=credentials, default_query_job_config=default_config, **kw)
default_config = kw.pop("default_query_job_config", bigquery.QueryJobConfig())
default_config.default_dataset = f"{project}.{dataset}"
self._client = bigquery.Client(project=project, credentials=credentials, default_query_job_config=default_config, **kw)

Comment thread dev/dev.env
Comment on lines +15 to +16
GOOGLE_APPLICATION_CREDENTIALS=bq.sa
DATADIFF_BIGQUERY_URI=bigquery://dms-analytics-v2-data-diff/test
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

Hardcoding specific project IDs (dms-analytics-v2-data-diff) and local file paths for credentials (bq.sa) in a shared environment file reduces portability and can lead to configuration issues for other contributors. It is recommended to use placeholders or provide a .env.example file instead.

@kantselovich kantselovich changed the title Bq issue 38 BQ Tests - Issue 38 Apr 28, 2026
@ramyamasani ramyamasani changed the title BQ Tests - Issue 38 Issue#38 Add targeted MySQL↔BigQuery CI coverage May 8, 2026
@ramyamasani ramyamasani force-pushed the bq_issue_38 branch 2 times, most recently from 28c8207 to 823ec30 Compare May 11, 2026 19:10
ram.yamasani@reachlocal.com added 2 commits May 11, 2026 12:34
ram.yamasani@reachlocal.com added 2 commits May 11, 2026 17:05
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.

Add targeted MySQL↔BigQuery CI coverage

2 participants