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

Infinispan - Support caching annotations #25300

Merged
merged 1 commit into from
Sep 26, 2022
Merged

Infinispan - Support caching annotations #25300

merged 1 commit into from
Sep 26, 2022

Conversation

karesti
Copy link
Member

@karesti karesti commented May 2, 2022 •
edited
Loading

Caching annotations @CacheInvalidate @CacheInvalidateAll and @CacheResult

https://issues.redhat.com/browse/ISPN-13062

@karesti karesti requested a review from wburns May 2, 2022 14:08
@quarkus-bot quarkus-bot bot added area/dependencies Pull requests that update a dependency file area/documentation area/infinispan Infinispan labels May 2, 2022
@gsmet
Copy link
Member

gsmet commented May 2, 2022

@karesti thanks but could you split that in two different commits? If we have to backport an Infinispan upgrade, I would like to be able to do it easily. Thanks.

@karesti
Copy link
Member Author

karesti commented May 2, 2022 •
edited
Loading

Infinispan Version Update #25306

@karesti karesti force-pushed the infinispan-caching-annotations branch 2 times, most recently from 839c66f to 7e2d9d5 Compare May 3, 2022 10:34
@karesti karesti changed the title Infinispan Client Updates Infinispan Client Caching annotations May 3, 2022
@gsmet gsmet changed the title Infinispan Client Caching annotations Infinispan - Support caching annotations May 3, 2022
@gsmet gsmet requested a review from gwenneg May 3, 2022 11:48
@gwenneg gwenneg self-assigned this May 3, 2022
gwenneg
gwenneg reviewed May 4, 2022
View reviewed changes
Copy link
Member

@gwenneg gwenneg left a comment

Choose a reason for hiding this comment

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

Here's a review focused on the code inspired/copied from quarkus-cache. Please double check the Infinispan-related code as it's not something I know in details 😄

...nt/deployment/src/main/java/io/quarkus/infinispan/client/deployment/CacheNamesBuildItem.java Outdated Show resolved Hide resolved
.../main/java/io/quarkus/infinispan/client/deployment/cache/exception/ClassTargetException.java Outdated Show resolved Hide resolved
...va/io/quarkus/infinispan/client/deployment/cache/exception/PrivateMethodTargetException.java Outdated Show resolved Hide resolved
...a/io/quarkus/infinispan/client/deployment/cache/exception/VoidReturnTypeTargetException.java Outdated Show resolved Hide resolved
...loyment/src/main/java/io/quarkus/infinispan/client/deployment/InfinispanClientProcessor.java Show resolved Hide resolved
...lient/runtime/src/main/java/io/quarkus/infinispan/client/runtime/cache/CacheInterceptor.java Show resolved Hide resolved
...lient/runtime/src/main/java/io/quarkus/infinispan/client/runtime/cache/CacheInterceptor.java Outdated Show resolved Hide resolved
docs/src/main/asciidoc/infinispan-client.adoc Show resolved Hide resolved
docs/src/main/asciidoc/infinispan-client.adoc Outdated Show resolved Hide resolved
...t/runtime/src/main/java/io/quarkus/infinispan/client/runtime/cache/InfinispanGetWrapper.java Outdated Show resolved Hide resolved
@karesti
Copy link
Member Author

karesti commented May 17, 2022

@gwenneg I made locally the small review changes and I will check the 2 PR you mentioned to report the changes here too. I ping you once is ready

@karesti karesti added this to the 2.10 - main milestone May 17, 2022
@karesti karesti marked this pull request as draft May 17, 2022 08:06
@karesti karesti force-pushed the infinispan-caching-annotations branch from 7e2d9d5 to 345079d Compare May 17, 2022 08:06
@gsmet gsmet removed this from the 2.10.0.CR1 milestone Jun 9, 2022
@geoand
Copy link
Contributor

geoand commented Jun 28, 2022

Hey,

What's the status of this?
This would be a super interesting feature and something that has been requested from the community.

@karesti
Copy link
Member Author

karesti commented Jun 29, 2022

@geoand I've been sidetracked too much recently, I need to report the changes reported by @gwenneg

@geoand
Copy link
Contributor

geoand commented Jun 29, 2022

Gotcha, thanks for the update

@karesti
Copy link
Member Author

karesti commented Jun 29, 2022

@geoand give me some days to get back to it properly. Sorry again

@geoand
Copy link
Contributor

geoand commented Jun 29, 2022

NP!

@geoand geoand closed this Jun 29, 2022
@quarkus-bot quarkus-bot bot added the triage/invalid This doesn't seem right label Jun 29, 2022
@geoand geoand reopened this Jun 29, 2022
@karesti karesti force-pushed the infinispan-caching-annotations branch 4 times, most recently from dc23fa4 to d65af68 Compare September 8, 2022 08:38
@karesti
Copy link
Member Author

karesti commented Sep 8, 2022

@wburns ready for a review round

wburns
wburns reviewed Sep 14, 2022
View reviewed changes
Copy link
Member

@wburns wburns left a comment

Choose a reason for hiding this comment

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

Just a couple more comments, otherwise looks good to me.

...t/runtime/src/main/java/io/quarkus/infinispan/client/runtime/cache/InfinispanGetWrapper.java Outdated Show resolved Hide resolved
...t/runtime/src/main/java/io/quarkus/infinispan/client/runtime/cache/InfinispanGetWrapper.java Outdated Show resolved Hide resolved
@karesti karesti force-pushed the infinispan-caching-annotations branch 2 times, most recently from 22bd544 to 47981d5 Compare September 15, 2022 14:14
@karesti karesti force-pushed the infinispan-caching-annotations branch from 47981d5 to 4fa2642 Compare September 15, 2022 14:47
@karesti
Copy link
Member Author

karesti commented Sep 15, 2022

@wburns changes reported
@geoand

wburns
wburns approved these changes Sep 15, 2022
View reviewed changes
Copy link
Member

@wburns wburns left a comment

Choose a reason for hiding this comment

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

LGTM from the ISPN side

@quarkus-bot

This comment has been minimized.

Sign in to view
@geoand
Copy link
Contributor

geoand commented Sep 15, 2022

Test failures seem related

@karesti karesti force-pushed the infinispan-caching-annotations branch from 4fa2642 to 5fe34b7 Compare September 16, 2022 15:55
@quarkus-bot

This comment has been minimized.

Sign in to view
@karesti karesti force-pushed the infinispan-caching-annotations branch from 5fe34b7 to 2774c3b Compare September 22, 2022 08:05
gwenneg
gwenneg approved these changes Sep 23, 2022
View reviewed changes
Copy link
Member

@gwenneg gwenneg left a comment

Choose a reason for hiding this comment

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

I added a minor comment below.

LGTM

...ntime/src/main/java/io/quarkus/infinispan/client/runtime/cache/SynchronousInfinispanGet.java Outdated Show resolved Hide resolved
@karesti karesti force-pushed the infinispan-caching-annotations branch from 2774c3b to e3b9ae6 Compare September 26, 2022 14:05
@karesti
Copy link
Member Author

karesti commented Sep 26, 2022

@geoand this seems validated

@geoand
Copy link
Contributor

geoand commented Sep 26, 2022

Cool! I'll let @gwenneg merge this

@gwenneg gwenneg merged commit 8b2acbe into quarkusio:main Sep 26, 2022
@quarkus-bot quarkus-bot bot added this to the 2.14 - main milestone Sep 26, 2022
@karesti karesti deleted the infinispan-caching-annotations branch September 27, 2022 14:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@geoand geoand geoand left review comments

@mkouba mkouba mkouba left review comments

@gwenneg gwenneg gwenneg approved these changes

@wburns wburns wburns approved these changes

Assignees

@wburns wburns

@gwenneg gwenneg

Labels
area/dependencies Pull requests that update a dependency file area/documentation area/infinispan Infinispan release/noteworthy-feature
Projects
None yet
Milestone
2.14.0.CR1
Development

Successfully merging this pull request may close these issues.
6 participants
@karesti @gsmet @geoand @gwenneg @wburns @mkouba