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.

Sign up for GitHub

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

Jump to bottom

ISPN-15211 Listeners have no way to ignore a previous old value #11358

Merged
merged 1 commit into from Oct 18, 2023
Merged

ISPN-15211 Listeners have no way to ignore a previous old value #11358

merged 1 commit into from Oct 18, 2023

Conversation

wburns
Copy link
Member

@wburns wburns commented Oct 4, 2023

@wburns wburns added the 14.0.x Annotate a PR with this label if you want it to be backported to the 14.0.x branch label Oct 4, 2023
@ryanemerson ryanemerson self-requested a review October 5, 2023 08:07
ryanemerson
ryanemerson approved these changes Oct 5, 2023
View reviewed changes
Copy link
Contributor

@ryanemerson ryanemerson left a comment

Choose a reason for hiding this comment

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

LGTM, just waiting on another CI run as the first one aborted.

@wburns wburns added the Urgent! label Oct 5, 2023
@wburns
Copy link
Member Author

wburns commented Oct 5, 2023

The first failure was due to GraalVM compiling our CLI which failed and the second was due to hibernate 2LC failing :( Both appear unrelated, I am restarting the build again.

@wburns wburns force-pushed the ISPN-15211_listener_prevvalue branch from 57be981 to f72def4 Compare October 5, 2023 13:57
@wburns
Copy link
Member Author

wburns commented Oct 5, 2023

Updated, tests pass with the change as well.

jabolina
jabolina approved these changes Oct 5, 2023
View reviewed changes
Copy link
Member

@jabolina jabolina left a comment

Choose a reason for hiding this comment

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

LGTM. Waiting for CI.

@wburns
Copy link
Member Author

wburns commented Oct 5, 2023

This is causing some failures, looking closer.

@wburns
Copy link
Member Author

wburns commented Oct 5, 2023

The problem is https://github.com/infinispan/infinispan/blob/main/server/hotrod/src/main/java/org/infinispan/server/hotrod/ClientListenerRegistry.java#L415 which I need to look into further, I was never a fan of having this in the first place tbh.

@wburns wburns added On Hold and removed Urgent! labels Oct 5, 2023
@wburns wburns force-pushed the ISPN-15211_listener_prevvalue branch from b38e242 to 7343ea7 Compare October 6, 2023 02:09
@tristantarrant
Copy link
Member

I think there are still some related, non-flaky failures

@wburns
Copy link
Member Author

wburns commented Oct 9, 2023

Correct, there is at least one but possibly two to four that I need to look into. CI was having trouble getting a build in the first place to confirm 😢

@wburns wburns force-pushed the ISPN-15211_listener_prevvalue branch from 7343ea7 to 2342181 Compare October 9, 2023 22:13
@wburns
Copy link
Member Author

wburns commented Oct 9, 2023

The two tests should be fixed now.

@wburns wburns removed On Hold 14.0.x Annotate a PR with this label if you want it to be backported to the 14.0.x branch labels Oct 17, 2023
@wburns
Copy link
Member Author

wburns commented Oct 17, 2023 •
edited

I don't think we want this in 14.0 after all, I will add a sentence to the upgrading guide to notify users who are using 15.

@wburns wburns force-pushed the ISPN-15211_listener_prevvalue branch from 2342181 to b5fa8d5 Compare October 17, 2023 17:37
@ryanemerson ryanemerson merged commit 5d42c97 into infinispan:main Oct 18, 2023
3 of 4 checks passed
@ryanemerson
Copy link
Contributor

Thanks @wburns

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ryanemerson ryanemerson ryanemerson approved these changes

@jabolina jabolina jabolina approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
4 participants
@wburns @tristantarrant @ryanemerson @jabolina