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

DynamicStrategy RefreshFrequency is hardcoded to 5s #175

Closed
patcher-ms opened this issue Sep 8, 2023 · 5 comments
Closed

DynamicStrategy RefreshFrequency is hardcoded to 5s #175

patcher-ms opened this issue Sep 8, 2023 · 5 comments
Labels

Comments

@patcher-ms
Copy link
Contributor

Describe the bug
I noticed that if even if change the refresh frequency in the configuration the template refreshes after 5 seconds. I believe this is due to this value being hardcoded to 5s

RefreshFrequency: 5 * time.Second,

Context

  • Sablier version: [e.g. 1.4.0-beta.4]
  • Provider: [e.g. docker]
  • Reverse proxy: [e.g. traefik v3.0]
  • Sablier running inside a container? Yes

Expected behavior
I would expect that changing Sablier's configuration

strategy:
  dynamic:
    default-refresh-frequency: 1s

or setting the refresh frequency in a label (e.g. traefik.http.middlewares.grafana.plugin.sablier.dynamic.refreshFrequency=10s) would result in a response with the refresh frequency I configured.

Additional context
All the other values I configured are picked up correctly so I don't this this is a misconfiguration on my side.
I am not using health checks on the Docker container which could hide the issue since an health check would prevent a pod from being detected as up until the service is actually ready.

@patcher-ms patcher-ms added the bug Something isn't working label Sep 8, 2023
@acouvreur
Copy link
Owner

Will look it up, seems easy to fix

acouvreur added a commit that referenced this issue Sep 11, 2023
The value was using a hardcoded 5s

Fixes #175
@acouvreur
Copy link
Owner

🎉 This issue has been resolved in version 1.4.0-beta.6 🎉

The release is available on:

Your semantic-release bot 📦🚀

@patcher-ms
Copy link
Contributor Author

Thanks for looking into this so quickly.

Unfortunately I am still having a couple issues:

@acouvreur
Copy link
Owner

acouvreur commented Sep 13, 2023

I'll take a look, but the value should be retrieved from the configuration first as a default value, and then overwritten by the body

@acouvreur
Copy link
Owner

🎉 This issue has been resolved in version 1.4.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

When branches are created from issues, their pull requests are automatically linked.

2 participants