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

Make caching more easily optional #28

Closed
UmanShahzad opened this issue Jan 12, 2021 · 4 comments · Fixed by #35
Closed

Make caching more easily optional #28

UmanShahzad opened this issue Jan 12, 2021 · 4 comments · Fixed by #35

Comments

@UmanShahzad
Copy link
Contributor

Right now caching can be modified by a settings key to choose your own cache implementation.

However it should be easier to disable caching (it should really default to disabling, but that would be a breaking change); currently that requires creating a custom cache implementation which simply does nothing.

We can either create such an implementation that does nothing, or have a new key like cache_disabled and update cache-using code to check if the cache exists before using it.

@UmanShahzad
Copy link
Contributor Author

Real use case for this is the Laravel SDK: it uses Laravel's own caching facilities already, and is apparently also using the PHP SDK's caching facility, which is completely redundant.

@coderholic
Copy link
Member

it uses Laravel's own caching facilities already, and is apparently also using the PHP SDK's caching facility, which is completely redundant

Our laravel sdk? Shouldn't we just remove caching from that completely?

@UmanShahzad
Copy link
Contributor Author

UmanShahzad commented Jan 12, 2021

@coderholic Doing that would then require exposing the caching interfaces of the PHP SDK to the user who's working with the Laravel SDK, if the Laravel user wants control over caching at all. We could do that or wrap all those interfaces with new ones.

But more importantly, Laravel has some framework-level caching facilities, which presumably most users with caching going on are using, and the Laravel SDK-level caching is delegating to that. It's just more sensible to use Laravel's caching facilities when needing to cache stuff in a Laravel library of any kind, if we can do that. Presumably this is why the original author of the Laravel SDK implemented it that way; more idiomatic/etc for that particular framework.

@UmanShahzad
Copy link
Contributor Author

Will be available in v2.3.0.

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 a pull request may close this issue.

2 participants