8000 fix: additional consolidation / refinement of argument parsing by shawkins · Pull Request #31448 · keycloak/keycloak · GitHub
[go: up one dir, main page]

Skip to content

fix: additional consolidation / refinement of argument parsing - #31448

Merged
mabartos merged 1 commit into
keycloak:mainfrom
shawkins:iss26339
Jul 24, 2024
Merged

mabartos merged 1 commit into
keycloak:mainfrom
shawkins:iss26339

Conversation

@shawkins
@shawkins shawkins commented Jul 19, 2024 •
Copy link
Copy Markdown
Contributor

closes: #26339 #31578

Took another pass over the various parsing we are doing to make another consolidation. It is possible that we could do the initial parsing / filtering to a better structure than just a list of strings to avoid the key parsing in subsequent passes. However that changes quite a bit of logic so it doesn't seem worth it.

closes: keycloak#26339

Signed-off-by: Steve Hawkins <shawkins@redhat.com>
@shawkins
Copy link
Copy Markdown
Contributor Author

@vmuzikar @mabartos @Pepo48 I noticed that this also fixes a bug - the previous refinements to the parsing logic were excluding --spi... value options (without an '=') from being picked up by the Config. This was not intentional, just a side effect of the focus on our primary options. So this can be considered / narrowed for backport as well.

@mabartos mabartos left a comment •
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@shawkins Much cleaner! And good job for spotting the bug around spi props values. Agree to backport the bug fix.

For better addressing, it'd be good to create a separate issue for the bug and reference also this PR there. WDYT?

@shawkins
Copy link
Copy Markdown
Contributor Author

For better addressing, it'd be good to create a separate issue for the bug and reference also this PR there. WDYT?

Captured as #31578

@vmuzikar vmuzikar linked an issue Jul 24, 2024 that may be closed by this pull request
stianst pushed a commit to stianst/keycloak that referenced this pull request Jul 25, 2024
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.

separated --spi options not picked up by the smallrye config Secondary parsing of cli arguments

2 participants

0