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] ConfigurationDataUtils does not throw exception on unknown properties anymore #23792

Open
2 of 3 tasks
alpreu opened this issue Dec 28, 2024 · 4 comments
Open
2 of 3 tasks
Labels
type/bug The PR fixed a bug or issue reported a bug

Comments

@alpreu
Copy link
Contributor

alpreu commented Dec 28, 2024

Search before asking

  • I searched in the issues and found nothing similar.

Read release policy

  • I understand that unsupported versions don't get bug fixes. I will attempt to reproduce the issue on a supported version of Pulsar client and Pulsar broker.

Version

Affected LTS versions are:
pulsar-client-all:3.0.7
pulsar-client-all:4.0.1

Minimal reproduce step

GitHub repository for reproducing the issue:
https://github.com/alpreu/pulsar-issue-23792 (Using Pulsar 3.0.7, no exception thrown)
https://github.com/alpreu/pulsar-issue-23792/tree/pulsar-3.0.6 (Using Pulsar 3.0.6, exception thrown as expected)

or alternatively,

Create a configuration HashMap containing unknown key-value pairs, then try to load it like this:

ClientConfigurationData conf =  ConfigurationDataUtils.loadData(
                configuration, new ClientConfigurationData(), ClientConfigurationData.class);

What did you expect to see?

RuntimeException with "Failed to load config into existing configuration data" to be thrown

What did you see instead?

No exception is thrown, the ClientConfigurationData object is instantiated

Anything else?

No response

Are you willing to submit a PR?

  • I'm willing to submit a PR!
@alpreu alpreu added the type/bug The PR fixed a bug or issue reported a bug label Dec 28, 2024
@pdolif
Copy link
Contributor

pdolif commented Dec 28, 2024

I think it is related to #22541 / c0c01c2.
There, @JsonIgnoreProperties(ignoreUnknown = true) was added to ClientConfigurationData.

@alpreu
Copy link
Contributor Author

alpreu commented Dec 29, 2024

I think it is related to #22541 / c0c01c2. There, @JsonIgnoreProperties(ignoreUnknown = true) was added to ClientConfigurationData.

Yes, looks like it. @lhotari was this an intentional change?

@lhotari
Copy link
Member

lhotari commented Dec 29, 2024

I think it is related to #22541 / c0c01c2. There, @JsonIgnoreProperties(ignoreUnknown = true) was added to ClientConfigurationData.

Yes, looks like it. @lhotari was this an intentional change?

@alpreu Yes, it was intentional at the time. I'll have to dig the reason.
What's the actual problem that it's causing? Please share more context about your use case. What makes your use case dependent on throwing an exception?

@alpreu
Copy link
Contributor Author

alpreu commented Jan 1, 2025

I think it is related to #22541 / c0c01c2. There, @JsonIgnoreProperties(ignoreUnknown = true) was added to ClientConfigurationData.

Yes, looks like it. @lhotari was this an intentional change?

@alpreu Yes, it was intentional at the time. I'll have to dig the reason. What's the actual problem that it's causing? Please share more context about your use case. What makes your use case dependent on throwing an exception?

It's not a big problem in that sense but having the exception in place as nice for us because we use plain Maps to configure connections, so when a user misspelled a parameter we could easily detect that, while now it would silently be discarded and they might wonder why their connection does not work

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/bug The PR fixed a bug or issue reported a bug
Projects
None yet
Development

No branches or pull requests

3 participants