-
Notifications
You must be signed in to change notification settings - Fork 48
TTL : Allow different cache strategy (exemple: greedyCacheStrategy) #137
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
Conversation
|
Could you say more about why you need to use a
The LaunchDarkly service endpoints do provide cache-related response headers, which are determined by the TTL setting on the LD dashboard. A cache that deliberately ignores those headers is contrary to the intended use. |
|
My bad... it works on a new created environment with the headers and the TTL defined. Thanks you for your reactivity and sorry for that PR |
|
No problem - glad it's working. |
|
@eli-darkly, have there been any other reports of this issue recently? i set the TTL on the environment to be 60 minutes, and checked outbound traffic from our container, and I can see it hits the app.launchdarkly.com service to retrieve feature flags with every request. client initialization looks like and there is nothing in the logs indicating the guzzle middleware package is missing. i even tested the next day in case there was some lag before environment TTL values were updated. i also took a look at what the above post said, and tried creating a new test environment and setting the TTL to a non-zero value when we created it (rather than creating an environment and updating it later down the line. the result was the same). when using the standard PublicCacheStrategy included in the sdk, but with a Flysystem cache storage, i can see the cached value looks something like
after forking the project and changing the GuzzleFeatureRequester to use a GreedyCacheStrategy, our service makes the initial request to get the feature flags, but subsequent requests to app.launchdarkly.com do not exist (as expected). do you have any suggestions? we are considering just forking the project and using the GreedyCacheStrategy. |
|
We haven't had any other reports of this issue, but the original reporter of this issue seems to have found that it was not actually a problem, so it's hard for me say whether what you're talking about is the same thing. However, similarly to what I discussed with the original reporter, it's unclear to me why you want to use GreedyCacheStrategy with this. What are you trying to accomplish? |
|
It sounds like you are saying that caching currently is not working at all, and that using GreedyCacheStrategy was just one approach you had thought of trying to work around this— is that right? |
|
I'm not able to test against your environment in particular since I don't know your SDK key (please don't post it here! if we do need to do any specific inspection of your environment, then we should move this conversation to a non-public support ticket), but when I test against an environment I created, I see the expected |
|
Also, it looks like you are using fairly old versions of your dependencies. The SDK has not been tested with Guzzle 6.x, or with guzzle-cache-middleware 1.x (v1.4.0 is over six years old), so it's hard for us to guarantee that those versions will work correctly. Our suggested versions as defined in our |
|
yes, the idea is to implement the GreedyCacheStrategy as a means of having some sort of forced caching, since we cannot get it to work properly with the PublicCacheStrategy. when sending the curl request, there cache-control with max-age is there, so it seems like somethings going wrong elsewhere in the pipeline. unfortunately we cannot use the suggested versions of guzzle and guzzle-cache-middleware, due to our PHP version being pretty old. i will open a support ticket and continue conversation there. thanks. |
|
OK. Please note that version 3.9.1 of our SDK will become unsupported too, a little over a month from now, per our EOL policy. I'm assuming that the reason you're still using that version is that you require compatibility with PHP 7.2 or lower, and I know sometimes there are obstacles to migrating these things, but we do need to be able to drop support for language versions that are no longer supported by their makers. |
I have to define my own TTL cache (via guzzle) so I need to use a different CacheStrategyInterface (GreedyCacheStrategy and not PublicCacheStrategy)
Example of use :
Can you accept this PR as soon as possible please ? (we need to put it in production)