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

bug: fix unmarshal consumer identity with empty value #1057

Conversation

fabianburth
Copy link
Contributor

@fabianburth fabianburth commented Nov 7, 2024

What this PR does / why we need it

Fix empty identity value regression.

The regression was introduced after this PR adding a custom unmarshaler for consumer identities which allows to specify values other than string (e.g. port: 5000), even though consumer identity is a map[string]string.

Which issue(s) this PR fixes

@fabianburth fabianburth requested a review from a team as a code owner November 7, 2024 09:25
Copy link
Contributor

@frewilhelm frewilhelm left a comment

Choose a reason for hiding this comment

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

Do you have any examples in which a consumer identity has an empty value and we don't want to throw an error or similar? It sounds odd to have a valid key but empty value^^

@fabianburth fabianburth force-pushed the unmarshal-consumer-identity branch from 809820f to 4c5824c Compare November 7, 2024 09:56
The regression was introduced after adding a custom unmarshaler for consumer identities which allows to specify values other than string (e.g. port: 5000), even though consumer identity is a map[string]string.
@fabianburth fabianburth force-pushed the unmarshal-consumer-identity branch from 4c5824c to 8bb36ae Compare November 7, 2024 14:06
@fabianburth fabianburth merged commit 37c6514 into open-component-model:main Nov 7, 2024
17 checks passed
@fabianburth fabianburth deleted the unmarshal-consumer-identity branch November 7, 2024 14:52
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.

2 participants