Skip to content

Make ReadFromBigQueryRequest id more randomized#30156

Merged
ahmedabu98 merged 3 commits intoapache:masterfrom
ahmedabu98:query_job_uuid
Jan 31, 2024
Merged

Make ReadFromBigQueryRequest id more randomized#30156
ahmedabu98 merged 3 commits intoapache:masterfrom
ahmedabu98:query_job_uuid

Conversation

@ahmedabu98
Copy link
Copy Markdown
Contributor

Request ID is just a random number between (0, 100,000). For long streaming jobs, this will inevitably create requests with identical ID's. This becomes a problem because query jobs are created from these IDs and BQ ignores identical job names.

@github-actions
Copy link
Copy Markdown
Contributor

Assigning reviewers. If you would like to opt out of this review, comment assign to next reviewer:

R: @jrmccluskey for label python.
R: @bvolpato for label io.

Available commands:

  • stop reviewer notifications - opt out of the automated review tooling
  • remind me after tests pass - tag the comment author after tests pass
  • waiting on author - shift the attention set back to the author (any comment or push by the author will return the attention set to the reviewers)

The PR bot will only process comments in the main thread (not review comments).

@@ -2926,7 +2928,7 @@ def __init__(
self.validate()

# We use this internal object ID to generate BigQuery export directories.
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.

Shall we update this code comment?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Nothing changed really, the comment is still valid. I guess we can also include that this is used to create job names

@liferoad
Copy link
Copy Markdown
Contributor

Since the fix is simple, if we do RC2, shall we cherry-pick this one? cc @lostluck

@lostluck
Copy link
Copy Markdown
Contributor

Since the fix is simple, if we do RC2, shall we cherry-pick this one? cc @lostluck

If it's a regression, it's a case for an RC2. Otherwise, I'm happy to fast track a simple fix in case we do need an RC2.

@ahmedabu98
Copy link
Copy Markdown
Contributor Author

I'm happy to fast track a simple fix in case we do need an RC2.

Sounds good! Will create a CP if that happens

@ahmedabu98 ahmedabu98 merged commit 41dee46 into apache:master Jan 31, 2024
lostluck added a commit that referenced this pull request Feb 5, 2024
… randomized (#30217)

* make ReadFromBigQueryRequest id more randomized

* lint

* update comment

---------

Co-authored-by: Ahmed Abualsaud <ahmedabualsaud@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants