Fix bq load for clashing copy job names#39107
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses an issue where BigQuery copy job names could clash during file load operations. By replacing the index-based naming strategy with a deterministic hash derived from the chunk contents, the system ensures that concurrent or repeated copy jobs maintain unique identifiers, thereby improving reliability in distributed execution environments. Highlights
New Features🧠 You can now enable Memory (public preview) to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
![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 updates the BigQuery file loads logic in the Python SDK to generate chunk job names using a SHA256 hash of the chunk contents instead of an index-based suffix. The review feedback recommends renaming the 'list' parameter in the new '_bq_uuid_list' helper function to avoid shadowing the built-in type, and replacing '%' string formatting with f-strings for better consistency and modern Python standards.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| def _bq_uuid_list(list): | ||
| checksum = hashlib.sha256() | ||
| for item in list: | ||
| checksum.update(item.encode('utf-8')) | ||
| # separator | ||
| checksum.update(b'\x00') | ||
| return checksum.hexdigest() |
There was a problem hiding this comment.
Avoid using the built-in name list as a parameter name, as it shadows the built-in list type. It is better to use a name like items or lst to prevent potential bugs and maintain readability.
| def _bq_uuid_list(list): | |
| checksum = hashlib.sha256() | |
| for item in list: | |
| checksum.update(item.encode('utf-8')) | |
| # separator | |
| checksum.update(b'\x00') | |
| return checksum.hexdigest() | |
| def _bq_uuid_list(items): | |
| checksum = hashlib.sha256() | |
| for item in items: | |
| checksum.update(item.encode('utf-8')) | |
| # separator | |
| checksum.update(b'\x00') | |
| return checksum.hexdigest() |
References
- Avoid shadowing built-in names like 'list' to prevent naming conflicts and maintain code clarity. (link)
| chunk_job_name = copy_job_name_base | ||
| if len(chunks) > 1: | ||
| chunk_job_name = f"{copy_job_name_base}_{i}" | ||
| chunk_job_name = '%s_%s' % (copy_job_name_base, _bq_uuid_list(chunk)) |
There was a problem hiding this comment.
Use f-strings instead of % formatting for consistency with the rest of the codebase and modern Python standards (PEP 498).
| chunk_job_name = '%s_%s' % (copy_job_name_base, _bq_uuid_list(chunk)) | |
| chunk_job_name = f"{copy_job_name_base}_{_bq_uuid_list(chunk)}" |
References
- Use f-strings (PEP 498) for string formatting to maintain consistency and readability. (link)
| chunk_job_name = copy_job_name_base | ||
| if len(chunks) > 1: | ||
| chunk_job_name = f"{copy_job_name_base}_{i}" | ||
| chunk_job_name = '%s_%s' % (copy_job_name_base, _bq_uuid_list(chunk)) |
There was a problem hiding this comment.
We have to be careful here. I remember bq load job name being deterministic is intended to avoid duplicate data on retry.
There was a problem hiding this comment.
If there is a commit causing these test failures (#38983 ?), consider revert it for the current release then fix forward, since release already cut yesterday and there is a risk potential problems not revealed if fix-forward with cherry-picks
stankiewicz
force-pushed
the
bq_copy_job_fix
branch
from
3f459bb to
a7e6f84
Compare
|
Checks are failing. Will not request review until checks are succeeding. If you'd like to override that behavior, comment |
2 participants
Fixes: https://github.com/apache/beam/actions/runs/28119469173/job/83267359072
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
addresses #123), if applicable. This will automatically add a link to the pull request in the issue. If you would like the issue to automatically close on merging the pull request, commentfixes #<ISSUE NUMBER>instead.CHANGES.mdwith noteworthy changes.See the Contributor Guide for more tips on how to make review process smoother.
To check the build health, please visit https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md
GitHub Actions Tests Status (on master branch)
See CI.md for more information about GitHub Actions CI or the workflows README to see a list of phrases to trigger workflows.