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

fix: use Symfony PSR-6 > PSR-16 cache adapter #110

Merged
merged 2 commits into from
Mar 15, 2021
Merged

fix: use Symfony PSR-6 > PSR-16 cache adapter #110

merged 2 commits into from
Mar 15, 2021

Conversation

darthf1
Copy link
Contributor

@darthf1 darthf1 commented Mar 14, 2021

Changes

With this change, Symfony's cache adapter is used instead of the one provided by this library.
https://symfony.com/doc/current/components/cache/psr6_psr16_adapters.html#using-a-psr-6-cache-object-as-a-psr-16-cache

References

From @evansims comment in #108 (comment):

Symfony has its own Psr16Adapter but for the life of me, I could not get it to work with the TraceableAdapter type Symfony is >using. Someone smarter than I might be able to figure that one out, in which case a PR would be most welcome.

Checklist

[x] I have read the Auth0 general contribution guidelines

[x] I have read the Auth0 Code of Conduct

[ ] All existing and new tests complete without errors

@darthf1 darthf1 requested a review from a team as a code owner March 14, 2021 19:17
@evansims
Copy link
Member

Hey, @darthf1, thanks for this! 👋 I tried going this Psr16Adapter route during the previous commit but I kept encountering exceptions around Psr16Adapter being unable to accept instances of Symfony\Component\Cache\Adapter\TraceableAdapter, and I could never figure out a workaround for it. I'll give this a try next week and see if this works better!

evansims
evansims previously approved these changes Mar 15, 2021
Copy link
Member

@evansims evansims left a comment

Choose a reason for hiding this comment

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

Looks good to me! No idea why this was throwing an error for me before but all appears well now, so all the better! Thanks for your contribution!

@darthf1
Copy link
Contributor Author

darthf1 commented Mar 15, 2021

Great! Do you want me to remove the helper and the related test as well as they're not used anymore?

@evansims
Copy link
Member

That would be great if you don't mind @darthf1

@darthf1
Copy link
Contributor Author

darthf1 commented Mar 15, 2021

@evansims no problem, done

Copy link
Member

@evansims evansims left a comment

Choose a reason for hiding this comment

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

Appreciate it! Just need to fix an issue with our status checks on the repo before I can merge, hang tight.

@evansims evansims closed this Mar 15, 2021
@evansims evansims reopened this Mar 15, 2021
@evansims evansims merged commit 0efae3c into auth0:master Mar 15, 2021
@evansims evansims added this to the 4.0.0 milestone Mar 23, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 18, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants