8000 fix: don't treat quarkus property changes as hard errors by shawkins · Pull Request #39459 · keycloak/keycloak · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

fix: don't treat quarkus property changes as hard errors #39459

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

Merged
merged 1 commit into from
May 5, 2025

Conversation

shawkins
Copy link
Contributor
@shawkins shawkins commented May 5, 2025

closes: #39450

It would be better to not directly modify the test resources conf diretory, but this change was a little more straight-forward than always creating / using a copy in the target directory.

mabartos
mabartos previously approved these changes May 5, 2025
Copy link
Contributor
@mabartos mabartos left a comment

Choose a reason for hiding this comment

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

LGTM

FYI - There's the access denied error on Windows.

@shawkins
Copy link
Contributor Author
shawkins commented May 5, 2025

FYI - There's the access denied error on Windows.

I'll have to copy to target instead then.

closes: keycloak#39450

Signed-off-by: Steve Hawkins <shawkins@redhat.com>
@mabartos
Copy link
Contributor
mabartos commented May 5, 2025

@shawkins AFAIK, you could use even the KeycloakDistribution.copyOrReplaceFile().

@shawkins
Copy link
Contributor Author
shawkins commented May 5, 2025

@shawkins AFAIK, you could use even the KeycloakDistribution.copyOrReplaceFile().

Do you mean move it to a more common spot? The server module is currently a dependency of the junit logic.

Copy link
Contributor
@vmuzikar vmuzikar left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the fix.

Should we also backport it?

@shawkins
Copy link
Contributor Author
shawkins commented May 5, 2025

LGTM, thanks for the fix.

Should we also backport it?

I don't see a risk, but it's also not a support scenario - and it's more likely users would be using environment variables, not the quarkus.properties for values that change at runtime.

@vmuzikar vmuzikar requested a review from mabartos May 5, 2025 15:52
@shawkins shawkins merged commit 071acce into keycloak:main May 5, 2025
80 checks passed
@mabartos
Copy link
Contributor
mabartos commented May 6, 2025

Do you mean move it to a more common spot? The server module is currently a dependency of the junit logic.

No, I just wanted to mention that we have already some kind of mechanism for copying files in the testsuite. But didn't check the requirements and stuff.

LGTM :)

InJoDave pushed a commit to InJoDave/keycloak that referenced this pull request May 6, 2025
)

closes: keycloak#39450

Signed-off-by: Steve Hawkins <shawkins@redhat.com>
shawkins added a commit to shawkins/keycloak that referenced this pull request May 7, 2025
)

closes: keycloak#39450

Signed-off-by: Steve Hawkins <shawkins@redhat.com>
shawkins added a commit to shawkins/keycloak that referenced this pull request May 16, 2025
)

closes: keycloak#39450

Signed-off-by: Steve Hawkins <shawkins@redhat.com>
(cherry picked from commit 071acce)
vmuzikar pushed a commit that referenced this pull request May 16, 2025
…9778)

closes: #39450


(cherry picked from commit 071acce)

Signed-off-by: Steve Hawkins <shawkins@redhat.com>
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.

quarkus runtime options are treated as buildtime options
3 participants
0