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

Config parsers should not discard existing configuration #17874

Open
pivovarit opened this issue Nov 22, 2020 · 4 comments
Open

Config parsers should not discard existing configuration #17874

pivovarit opened this issue Nov 22, 2020 · 4 comments
Assignees
Milestone

Comments

@pivovarit
Copy link
Contributor

pivovarit commented Nov 22, 2020

Configuration parsers accept external config objects(MemberDomConfigProcessor(boolean domLevel3, Config config) { ... } ) but discard some of their insides and overwrite them with Java objects defaults which lead to issues like:

To avoid this kind of issue, config objects should initialize all their nested configuration objects and parsers should merely modify desired entries instead of discarding existing config objects and replacing them with new ones - this avoids an undesired effect of config entries being reverted back to defaults after using config overwrite mechanism.

This is just an umbrella issue to help track the progress. Multiple PRs are needed. To be backported to 4.1.1

@pivovarit pivovarit self-assigned this Nov 22, 2020
@pivovarit pivovarit added this to the 4.1.1 milestone Nov 22, 2020
@pivovarit pivovarit modified the milestones: 4.1.1, 4.2 Nov 24, 2020
@mmedenjak
Copy link
Contributor

@pivovarit how much more do you assess is remaining? I see lots has been fixed already.

@pivovarit
Copy link
Contributor Author

Most issues on the member-side are covered. I'd say that only very specific corner cases remain, but these are unlikely to be touched when working with config overrides. However, there are still a couple of issues on the client-side and I'm actually working on them (want to submit a bigger PR for them)

@Holmistr
Copy link
Contributor

@pivovarit How is it going? We're getting closer and closer to the release, so if it's not really ready and won't be any time soon, we should move it out of 4.2 scope.

@pivovarit
Copy link
Contributor Author

With my new recent assignments, I have a hard time dedicating time to them. Keep in mind that it's mostly done, so we can easily release what was merged so far

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants