8000 Fix compilation error by pruivo · Pull Request #30416 · keycloak/keycloak · GitHub
[go: up one dir, main page]

Skip to content

Fix compilation error - #30416

Merged
pedroigor merged 1 commit into
keycloak:mainfrom
pruivo:t_fix_compilation_error
Jun 13, 2024
Merged

pedroigor merged 1 commit into
keycloak:mainfrom
pruivo:t_fix_compilation_error

Conversation

@pruivo
@pruivo pruivo commented Jun 13, 2024
Copy link
Copy Markdown
Member

No description provided.

@pruivo
pruivo requested a review from a team as a code owner June 13, 2024 17:33
@pruivo
pruivo commented Jun 13, 2024
Copy link
Copy Markdown
Member Author

Let's wait for the tests to run.

@pruivo
pruivo commented Jun 13, 2024
Copy link
Copy Markdown
Member Author

The Protostream pull request caused a conflict with #30381 and the main branch is not compiling anymore.

I'm surprised the test suite didn't fail with #30381 because the events are cast to RealmCacheInvalidationEvent. See:

https://github.com/keycloak/keycloak/blob/f44c5b64db67d125ccd4f6e75b63d440697e0e03/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/RealmCacheManager.java#L135

I've created new event 8000 s. Can @vramik @pedroigor review this PR, please? Thanks!

@ahus1
ahus1 commented Jun 13, 2024
Copy link
Copy Markdown
Member

@pruivo - I talked to @pedroigor and he would be ok to merge this now to fix the broken build and then discuss later. He'll be in contact with you.

@pedroigor pedroigor Jun 13, 2024 •
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

One of the reasons we added this method is to allow register invalidations on the realm cache for whatever entry we want. Therefore, avoid having methods like those below that are specific to organizations.

For instance, other similar methods like registerGroup|role|etc do not make much sense if you can have a single method like that one. We are not changing this in other places but for orgs, we would like to avoid adding methods in this interface.

Can we keep the previous contract?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yes, I can add a "generic" event and keep the old contract 👍

Signed-off-by: Pedro Ruivo <pruivo@redhat.com>
@pruivo
pruivo force-pushed the t_fix_compilation_error branch from f44c5b6 to b36232a Compare June 13, 2024 19:02
@pruivo
pruivo commented Jun 13, 2024
Copy link
Copy Markdown
Member Author

Thanks @ahus1 @pedroigor. I've updated the PR.

8000
@pedroigor pedroigor left a comment •
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks!

@pruivo
pruivo commented Jun 13, 2024
Copy link
Copy Markdown
Member Author

ps. didn't run the test suite locally, better wait for GHA report 👍

@pedroigor
pedroigor enabled auto-merge (rebase) June 13, 2024 19:37
@pedroigor
pedroigor merged commit 1b342e4 into keycloak:main Jun 13, 2024
@pruivo
pruivo deleted the t_fix_compilation_error branch June 13, 2024 19:46
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.

3 participants

0