Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Portable BQ] default null list to empty list #33919

Merged
merged 1 commit into from
Feb 11, 2025

Conversation

ahmedabu98
Copy link
Contributor

Fixes a problem where, in the DLQ, failed rows containing null values for list fields will fail to convert to Beam Rows.

@ahmedabu98
Copy link
Contributor Author

assign set of reviewers

Copy link
Contributor

github-actions bot commented Feb 7, 2025

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

R: @m-trieu for label java.
R: @shunping 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).

@liferoad
Copy link
Contributor

liferoad commented Feb 7, 2025

Can we cherry-pick this one? @jrmccluskey

@jrmccluskey
Copy link
Contributor

@liferoad RC1 is already out for a vote, so to get this in 2.63.0 we'd need to either find a regression that needs fixing in that RC or deem this fix important enough to move to an RC2 anyway

@ahmedabu98
Copy link
Contributor Author

I don't think it warrants a new RC since this isn't a new regression introduced in 2.63.0.

If there happens to be a new RC for a different reason, it'd be cool to include this, but up to @jrmccluskey's judgement

@jrmccluskey
Copy link
Contributor

I already have another PR cherry picked to the release branch in case we go to RC2, so feel free to open a cherry pick PR once this is merged

@jrmccluskey
Copy link
Contributor

Looks like we're heading to RC2 if you want to get this in @ahmedabu98

@ahmedabu98 ahmedabu98 merged commit 2b18c28 into apache:master Feb 11, 2025
15 checks passed
@ahmedabu98
Copy link
Contributor Author

Thanks for the heads up.
sorry, had to re-run the java precommit quite a bit because of flakes

ahmedabu98 added a commit to ahmedabu98/beam that referenced this pull request Feb 11, 2025
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