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

[WFCORE-6956] Wrong endpoint-name if the JBoss server name and 'jboss… #6137

Merged
merged 1 commit into from
Aug 22, 2024

Conversation

jbaesner
Copy link
Contributor

@github-actions github-actions bot added the deps-ok Dependencies have been checked, and there are no significant changes label Aug 21, 2024
….node.name' are both provided to the server

check for existence of Property key instead of the value on the
primordialProperties
@yersan yersan added the ready-for-merge This PR is ready to be merged and fulfills all requirements label Aug 22, 2024
@yersan yersan merged commit 8563703 into wildfly:main Aug 22, 2024
11 of 12 checks passed
@yersan
Copy link
Collaborator

yersan commented Aug 22, 2024

Thanks @jbaesner

@rachmatowicz
Copy link
Contributor

This commit really could have used a test case, which would have prevented the current error from getting through as well as demonstrating and explaining the expected behavior. I had a thorough look over it today and the change looks correct, but the logic for computing the values is not trivial, especially that it needs to work correctly (1) on a reload of the server and (2) in the case of management updates to system properties.

@yersan
Copy link
Collaborator

yersan commented Aug 26, 2024

@rachmatowicz makes sense to me, I've created this https://issues.redhat.com/browse/WFCORE-6959 to track it.

My feeling is the behavior of these values is correctly managed and the original bug and resolution were pretty clear to me, but yes, I agree we could have added a test case to cover (and explain) it for the future, thanks for pointing it out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deps-ok Dependencies have been checked, and there are no significant changes ready-for-merge This PR is ready to be merged and fulfills all requirements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants