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-7089] When setting a policy in the PolicyDefinitions do not a… #6279

Merged
merged 1 commit into from
Dec 16, 2024

Conversation

jamezp
Copy link
Member

@jamezp jamezp commented Dec 13, 2024

…ttempt to refresh the policy if it's null.

https://issues.redhat.com/browse/WFCORE-7089

@wildfly-ci
Copy link

Core -> Full Integration Build 14417 outcome was UNKNOWN using a merge of 13264d8
Summary: Canceled (Tests passed: 4057, ignored: 50; Step 5/6) Build time: 01:01:50

@wildfly-ci
Copy link

Core -> Full Integration Build 14119 outcome was UNKNOWN using a merge of 13264d8
Summary: Canceled (Tests passed: 1626, ignored: 31; Step 5/6) Build time: 00:59:01

@wildfly-ci
Copy link

Core -> WildFly Preview Integration Build 14199 outcome was UNKNOWN using a merge of 13264d8
Summary: Canceled (Tests passed: 1940, ignored: 18; Step 5/6) Build time: 00:44:36

@wildfly-ci

This comment was marked as off-topic.

@bstansberry bstansberry requested review from darranl and fjuma December 15, 2024 21:35
@bstansberry
Copy link
Contributor

@darranl or @fjuma Is this ok?

The NPE fix is clearly fine; I just want to make sure you don't see anything wrong with 'policy' being null in the first place.

@bstansberry
Copy link
Contributor

Well, I asked so I'll be patient a bit for an answer, but thinking more this seems fine. The 'policy' param is null because it's what was read from getPolicy() in service start, and on SE 24 there is no particular reason for that call to return anything but null.

@darranl
Copy link
Contributor

darranl commented Dec 16, 2024

@bstansberry yes this is fine, before Java 24 we would have always had a policy but 24 onwards we will never have a Policy - this service is sufficiently self contained there are not other reasons this would become null.

@darranl
Copy link
Contributor

darranl commented Dec 16, 2024

The test failures look unrelated, I was going to kick off CI but I see we have a big queue at the moment.

@yersan
Copy link
Collaborator

yersan commented Dec 16, 2024

@darranl test failures are old friends and unrelated, they are also failing with similar traces for other PR, the xerces one is recently, but unrelated. I adding a "ready-to-merge"

@yersan yersan added the ready-for-merge This PR is ready to be merged and fulfills all requirements label Dec 16, 2024
@yersan yersan merged commit 9a07bd9 into wildfly:main Dec 16, 2024
11 of 13 checks passed
@yersan
Copy link
Collaborator

yersan commented Dec 16, 2024

Thanks @jamezp @bstansberry @darranl

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
27.x Blocker 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.

5 participants