Skip to content

[AIBundle] Cache store configuration improved #293

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

Merged
merged 1 commit into from
Aug 15, 2025

Conversation

Guikingone
Copy link
Contributor

Q A
Bug fix? yes
New feature? no
Docs? yes
Issues #257
License MIT

Hi 👋🏻

The configuration wasn't complete enough to handle custom strategy / key for the cache store, here's the fix, tests are validated (both in the project and in a custom project too).

@Guikingone Guikingone force-pushed the store/cache_fix branch 2 times, most recently from 3680a9e to 987fd0a Compare August 9, 2025 16:37
@Guikingone Guikingone requested a review from chr-hertel August 9, 2025 17:23
@Guikingone Guikingone requested a review from chr-hertel August 11, 2025 07:10
@Guikingone Guikingone force-pushed the store/cache_fix branch 3 times, most recently from 001c006 to d0d9815 Compare August 12, 2025 07:39
@Guikingone Guikingone force-pushed the store/cache_fix branch 2 times, most recently from 220e3b6 to 45b6de4 Compare August 13, 2025 11:58
Copy link
Member

Choose a reason for hiding this comment

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

Let's remove the CacheItemPoolInterface then here completely 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hum nope, it's required for the static analysis, without the CacheItemPoolInterface, PHPStan isn't capable of resolving the methods from CIP.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The only way to allows it to resolve it is by adding:

private CacheInterface&CacheItemPoolInterface $cache,

@chr-hertel
Copy link
Member

Needs a rebase after #318

@chr-hertel
Copy link
Member

Thank you @Guikingone.

@chr-hertel chr-hertel merged commit 77fa8b6 into symfony:main Aug 15, 2025
8 checks passed
chr-hertel added a commit that referenced this pull request Aug 16, 2025
…-hertel)

This PR was merged into the main branch.

Discussion
----------

[Examples] Add symfony/cache as dependency for examples

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| Docs?         | no
| Issues        |
| License       | MIT

Related to #257 and #293

Commits
-------

e641c09 Add symfony/cache as dependency for examples
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working Status: Reviewed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants