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 Cache Extension #39836

Merged
merged 1 commit into from
Apr 23, 2024
Merged

Infinispan Cache Extension #39836

merged 1 commit into from
Apr 23, 2024

Conversation

karesti
Copy link
Member

@karesti karesti commented Apr 2, 2024 •
edited
Loading

  • Deprecates custom annotations
  • Implements the cache extension SPI

Closes #31896
Closes #29571

Depends on #40038

@karesti karesti requested a review from geoand April 2, 2024 14:29
@quarkus-bot quarkus-bot bot added area/dependencies Pull requests that update a dependency file area/devtools Issues/PR related to maven, gradle, platform and cli tooling/plugins area/documentation area/infinispan Infinispan labels Apr 2, 2024
@karesti karesti mentioned this pull request Apr 2, 2024
Open
@karesti karesti requested a review from wburns April 2, 2024 14:35
@quarkus-bot

This comment has been minimized.

Sign in to view
@quarkus-bot

This comment has been minimized.

Sign in to view
@karesti karesti force-pushed the infinispan-cache-extension branch from 4634de7 to ce98f06 Compare April 2, 2024 16:10
@quarkus-bot

This comment has been minimized.

Sign in to view
@quarkus-bot

This comment has been minimized.

Sign in to view
geoand
geoand reviewed Apr 3, 2024
View reviewed changes
Copy link
Contributor

@geoand geoand left a comment

Choose a reason for hiding this comment

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

I don't know a ton about the Infinispan implementation, but the integration with Quarkus seems to do what we are doing in Redis, so it makes sense

@karesti karesti force-pushed the infinispan-cache-extension branch from ce98f06 to 1e336be Compare April 3, 2024 09:20
@quarkus-bot

This comment has been minimized.

Sign in to view
@karesti karesti force-pushed the infinispan-cache-extension branch from 1e336be to 1af8976 Compare April 3, 2024 09:43
@quarkus-bot

This comment has been minimized.

Sign in to view
@quarkus-bot

This comment has been minimized.

Sign in to view
@karesti karesti force-pushed the infinispan-cache-extension branch from 1af8976 to 9599407 Compare April 3, 2024 10:07
@quarkus-bot

This comment has been minimized.

Sign in to view
@quarkus-bot

This comment has been minimized.

Sign in to view
@karesti karesti force-pushed the infinispan-cache-extension branch from 9599407 to 6d4e1bf Compare April 3, 2024 11:47
@quarkus-bot

This comment has been minimized.

Sign in to view
@quarkus-bot

This comment has been minimized.

Sign in to view
@karesti
Copy link
Member Author

karesti commented Apr 3, 2024

@gsmet Guillaume could your eyes give some light to this failure ? i've run the /update-extension-dependencies.sh script locally but there aren't more apparent changes needed and build still fails. I can't see what I'm missing here

@gsmet gsmet force-pushed the infinispan-cache-extension branch from 6d4e1bf to b85c6d5 Compare April 3, 2024 13:22
@quarkus-bot

This comment has been minimized.

Sign in to view
@quarkus-bot

This comment has been minimized.

Sign in to view
@quarkus-bot

This comment has been minimized.

Sign in to view
@github-actions GitHub Actions
Copy link

github-actions bot commented Apr 16, 2024 •
edited
Loading

🙈 The PR is closed and the preview is expired.

@quarkus-bot

This comment has been minimized.

Sign in to view
@quarkus-bot

This comment has been minimized.

Sign in to view
@karesti
Copy link
Member Author

karesti commented Apr 17, 2024

@cescoffier I pushed changes

@quarkus-bot

This comment has been minimized.

Sign in to view
@quarkus-bot

This comment has been minimized.

Sign in to view
cescoffier
cescoffier approved these changes Apr 21, 2024
View reviewed changes
...finispan-cache/deployment/src/test/java/io/quarkus/cache/infinispan/InfinispanCacheTest.java
@RegisterExtension
static final QuarkusUnitTest config = new QuarkusUnitTest()
.withEmptyApplication()
.withConfigurationResource("empty-application-infinispan-client.properties");
Copy link
Member

Choose a reason for hiding this comment

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

We may have a better way to create empty assets, but I can't remember the API.

...finispan-cache/deployment/src/test/java/io/quarkus/cache/infinispan/InfinispanCacheTest.java
Future<Uni<String>> thread1 = fork(() -> cache.get(id, key -> {
try {
barrier.await(10, TimeUnit.SECONDS);
barrier.await(10, TimeUnit.SECONDS);
Copy link
Member

Choose a reason for hiding this comment

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

I would add that explanation in the code - because you can be sure I would ask the same question in 1 month... or one week, or even maybe tomorrow.

@karesti
Copy link
Member Author

karesti commented Apr 22, 2024

@cescoffier I'm going to add the comment in the test and push.
Is there anything else preventing the merge at this point ?

@karesti
Infinispan Cache Extension
b6285a6 
* Deprecates custom annotations
* Implements the cache extension SPI
@quarkus-bot
Copy link

quarkus-bot bot commented Apr 22, 2024

Status for workflow Quarkus Documentation CI

This is the status report for running Quarkus Documentation CI on commit b6285a6.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

⚠️ There are other workflow runs running, you probably need to wait for their status before merging.

@quarkus-bot
Copy link

quarkus-bot bot commented Apr 22, 2024 •
edited by github-actions bot
Loading

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit b6285a6.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

You can consult the Develocity build scans.

Flaky tests - Develocity

⚙️ JVM Tests - JDK 17

📦 extensions/websockets-next/server/deployment

✖ io.quarkus.websockets.next.test.openconnections.OpenConnectionsTest.testOpenConnections - History

  • Condition with Lambda expression in io.quarkus.websockets.next.test.utils.WSClient was not fulfilled within 10 seconds. - org.awaitility.core.ConditionTimeoutException
org.awaitility.core.ConditionTimeoutException: Condition with Lambda expression in io.quarkus.websockets.next.test.utils.WSClient was not fulfilled within 10 seconds.
	at org.awaitility.core.ConditionAwaiter.await(ConditionAwaiter.java:167)
	at org.awaitility.core.CallableCondition.await(CallableCondition.java:78)
	at org.awaitility.core.CallableCondition.await(CallableCondition.java:26)
	at org.awaitility.core.ConditionFactory.until(ConditionFactory.java:1006)
	at org.awaitility.core.ConditionFactory.until(ConditionFactory.java:975)
	at io.quarkus.websockets.next.test.utils.WSClient.waitForMessages(WSClient.java:102)
	at io.quarkus.websockets.next.test.openconnections.OpenConnectionsTest.testOpenConnections(OpenConnectionsTest.java:69)

⚙️ JVM Tests - JDK 21

📦 extensions/websockets-next/server/deployment

✖ io.quarkus.websockets.next.test.openconnections.OpenConnectionsTest.testOpenConnections - History

  • Condition with Lambda expression in io.quarkus.websockets.next.test.utils.WSClient was not fulfilled within 10 seconds. - org.awaitility.core.ConditionTimeoutException
org.awaitility.core.ConditionTimeoutException: Condition with Lambda expression in io.quarkus.websockets.next.test.utils.WSClient was not fulfilled within 10 seconds.
	at org.awaitility.core.ConditionAwaiter.await(ConditionAwaiter.java:167)
	at org.awaitility.core.CallableCondition.await(CallableCondition.java:78)
	at org.awaitility.core.CallableCondition.await(CallableCondition.java:26)
	at org.awaitility.core.ConditionFactory.until(ConditionFactory.java:1006)
	at org.awaitility.core.ConditionFactory.until(ConditionFactory.java:975)
	at io.quarkus.websockets.next.test.utils.WSClient.waitForMessages(WSClient.java:102)
	at io.quarkus.websockets.next.test.openconnections.OpenConnectionsTest.testOpenConnections(OpenConnectionsTest.java:69)

@karesti
Copy link
Member Author

karesti commented Apr 23, 2024

@geoand hey, do you spot anything missing at this point preventing the merge ?

geoand
geoand approved these changes Apr 23, 2024
View reviewed changes
Copy link
Contributor

@geoand geoand left a comment

Choose a reason for hiding this comment

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

LGTM

@geoand geoand merged commit a3d1568 into quarkusio:main Apr 23, 2024
54 checks passed
@quarkus-bot quarkus-bot bot added this to the 3.11 - main milestone Apr 23, 2024
@quarkus-bot quarkus-bot bot added kind/bugfix kind/enhancement New feature or request labels Apr 23, 2024
@karesti karesti deleted the infinispan-cache-extension branch April 23, 2024 11:33
@karesti karesti mentioned this pull request Apr 26, 2024
Merged
@gtroitsk gtroitsk mentioned this pull request Aug 2, 2024
Merged
9 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wburns wburns wburns left review comments

@cescoffier cescoffier cescoffier approved these changes

@geoand geoand geoand approved these changes

Assignees
No one assigned
Labels
area/dependencies Pull requests that update a dependency file area/devtools Issues/PR related to maven, gradle, platform and cli tooling/plugins area/documentation area/infinispan Infinispan area/infra-automation anything related to CI, bots, etc. that are used to automated our infrastructure kind/bugfix kind/enhancement New feature or request release/noteworthy-feature triage/flaky-test
Projects
Quarkus Documentation
Status: Done
Milestone
3.11.0.CR1
4 participants
@karesti @geoand @cescoffier @wburns