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

Add Redis cache tests and bump Redis version #1524

Merged
merged 2 commits into from
Nov 21, 2023

Conversation

jedla97
Copy link
Member

@jedla97 jedla97 commented Nov 16, 2023

Summary

Hi, this PR main objective was cover quarkusio/quarkus#35680. As it was need standalone module I added some test which was overlapping between caffeine cache and spring cache. Test to cover 35680 is quteShouldThrowError. Tested manually the redis cache with rest of the api and I saw it cached in Redis (used redis-cli keys "*").

In standalone commit is update of Redis version (Quarkus dev mode using latest).

Please select the relevant options.

  • Bug fix (non-breaking change which fixes an issue)
  • Dependency update
  • Refactoring
  • Backport
  • New scenario (non-breaking change which adds functionality)
  • This change requires a documentation update
  • This change requires execution against OCP (use run tests phrase in comment)

Checklist:

  • Methods and classes used in PR scenarios are meaningful
  • Commits are well encapsulated and follow the best practices

@jedla97
Copy link
Member Author

jedla97 commented Nov 16, 2023

run tests

@michalvavrik
Copy link
Member

OCP failures are not related, problem is in our snapshot deployment job.

@jedla97 jedla97 requested a review from michalvavrik November 20, 2023 07:49
@michalvavrik
Copy link
Member

@jedla97 yesterday this looked good to me, but I need to go through it again. Please rebase on current and re-run OCP tests (there was some fix on our infra). Thanks

@jedla97
Copy link
Member Author

jedla97 commented Nov 20, 2023

run tests

Copy link
Member

@michalvavrik michalvavrik left a comment

Choose a reason for hiding this comment

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

This is very nice work, please add section to the README regarding new module. I'm also not psyched about adding this to the root modules considering OOM at native failures, but that should be fixed separately (for you are adding it next to caffeine, your PR is fine).

@jedla97
Copy link
Member Author

jedla97 commented Nov 21, 2023

run tests

@jedla97
Copy link
Member Author

jedla97 commented Nov 21, 2023

@michalvavrik I updated README + add the module to cache profile.

Copy link
Member

@michalvavrik michalvavrik left a comment

Choose a reason for hiding this comment

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

LGTM. We can fix the typo in a separate commit if you wish. This can go in as soon as CI is green.

README.md Outdated Show resolved Hide resolved
Most of the tests are reused from caffeine module anshould cover similar scenarios as spring che and caffeine cache.
@michalvavrik
Copy link
Member

@jedla97 let's not re-run OCP tests, I'll look to Jenkins on results of the ones you triggered previously. You can't cancel them from here.

@michalvavrik michalvavrik merged commit 92cb88c into quarkus-qe:main Nov 21, 2023
7 checks passed
@michalvavrik michalvavrik removed the triage/backport-3.2 RHBQ Ghost LTS release label Nov 21, 2023
@jedla97 jedla97 deleted the add-redis-cache branch December 10, 2024 12:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants