Skip to content

KAFKA-8659: SetSchemaMetadata SMT fails on records with null value and schema#7082

Merged
bbejeck merged 2 commits intoapache:trunkfrom
bfncs:kafka-8659
Mar 1, 2022
Merged

KAFKA-8659: SetSchemaMetadata SMT fails on records with null value and schema#7082
bbejeck merged 2 commits intoapache:trunkfrom
bfncs:kafka-8659

Conversation

@bfncs
Copy link
Copy Markdown
Contributor

@bfncs bfncs commented Jul 12, 2019

Make SetSchemaMetadata SMT ignore records with null value and valueSchema or key and keySchema.

The transform has been unit tested for handling null values gracefully while still providing the necessary validation for non-null values.

@bfncs
Copy link
Copy Markdown
Contributor Author

bfncs commented Sep 6, 2019

Any chance I can get some feedback on this PR? Is there anything wrong, should anything be corrected?

Copy link
Copy Markdown
Contributor

@kkonstantine kkonstantine left a comment

Choose a reason for hiding this comment

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

Thanks for the PR @bfncs !
Just a nit regarding imports but otherwise, LGTM

@bfncs
Copy link
Copy Markdown
Contributor Author

bfncs commented Apr 6, 2020

Thanks for reviewing, @kkonstantine. The PR is already quite old, had to merge master again and reorder, should be ok now.

@kkonstantine
Copy link
Copy Markdown
Contributor

retest this please

1 similar comment
@kkonstantine
Copy link
Copy Markdown
Contributor

retest this please

@hartmut-co-uk
Copy link
Copy Markdown

I also run into this, @bfncs are you able to pick this up again, or can I help?

@bfncs bfncs force-pushed the kafka-8659 branch 2 times, most recently from 0cc8537 to 5f9dd75 Compare January 11, 2022 21:17
@bfncs
Copy link
Copy Markdown
Contributor Author

bfncs commented Jan 13, 2022

I finally adopted the patch to the current trunk. The CI test run here failed though, while at least all tests from the related connect transforms package run for me locally. Can anyone please retrigger and check tests and provide guidance on how to continue, please? @kkonstantine maybe?

@bfncs
Copy link
Copy Markdown
Contributor Author

bfncs commented Jan 24, 2022

Does anyone have a pointer how we can proceed here?

@hartmut-co-uk
Copy link
Copy Markdown

the changes are small enough... but looking at other PRs seems they all are failing, too...

@bfncs
Copy link
Copy Markdown
Contributor Author

bfncs commented Feb 24, 2022

It would be nice to finally have this merged. Is there anything I could do to help?

@bbejeck
Copy link
Copy Markdown
Member

bbejeck commented Feb 24, 2022

It would be nice to finally have this merged. Is there anything I could do to help?

@bfncs kicked off the build again if we get green test runs I think we can merge this

\cc @kkonstantine

@bbejeck
Copy link
Copy Markdown
Member

bbejeck commented Feb 24, 2022

@bfncs sorry to ask this, but I just noticed the date of the last commit - do you mind doing a rebase? Apologies again for not noticing before

@bfncs
Copy link
Copy Markdown
Contributor Author

bfncs commented Feb 25, 2022

Thanks for picking this up, @bbejeck! I merged trunk into the feature branch again.

Copy link
Copy Markdown
Member

@bbejeck bbejeck left a comment

Choose a reason for hiding this comment

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

LGTM

@bbejeck
Copy link
Copy Markdown
Member

bbejeck commented Mar 1, 2022

Test failures are unrelated

@bbejeck bbejeck merged commit 14faea4 into apache:trunk Mar 1, 2022
@bbejeck
Copy link
Copy Markdown
Member

bbejeck commented Mar 1, 2022

Thanks, @bfncs for the contribution and your patience!

@bbejeck
Copy link
Copy Markdown
Member

bbejeck commented Mar 1, 2022

@bfncs I've also added you as a contributor on Jira so you'll be able to assign tickets to yourself in the future

bbejeck pushed a commit that referenced this pull request Mar 1, 2022
…7082)

Make SetSchemaMetadata SMT ignore records with null value and valueSchema or key and keySchema.

The transform has been unit tested for handling null values gracefully while still providing the necessary validation for non-null values.

Reviewers: Konstantine Karantasis<konstantine@confluent.io>, Bill Bejeck <bbejeck@apache.org>
@bfncs
Copy link
Copy Markdown
Contributor Author

bfncs commented Mar 1, 2022

Thanks for reviewing, merging and adding me to Jira, @bbejeck. 💐

bbejeck pushed a commit that referenced this pull request Mar 1, 2022
…7082)

Make SetSchemaMetadata SMT ignore records with null value and valueSchema or key and keySchema.

The transform has been unit tested for handling null values gracefully while still providing the necessary validation for non-null values.

Reviewers: Konstantine Karantasis<konstantine@confluent.io>, Bill Bejeck <bbejeck@apache.org>
@bbejeck
Copy link
Copy Markdown
Member

bbejeck commented Mar 1, 2022

Merged to trunk, 3.0 and 3.1 branches

jeffkbkim pushed a commit to confluentinc/kafka that referenced this pull request May 12, 2022
…pache#7082)

Make SetSchemaMetadata SMT ignore records with null value and valueSchema or key and keySchema.

The transform has been unit tested for handling null values gracefully while still providing the necessary validation for non-null values.

Reviewers: Konstantine Karantasis<konstantine@confluent.io>, Bill Bejeck <bbejeck@apache.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants