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

feat: aligned environment variables application with flagd provider spec #119

Merged
merged 2 commits into from
Feb 21, 2023

Conversation

skyerus
Copy link
Contributor

@skyerus skyerus commented Feb 17, 2023

This PR

Aligns environment variables with flagd provider spec: https://github.com/open-feature/flagd/blob/main/docs/other_resources/creating_providers.md?plain=1#L117
FLAGD_CACHING_DISABLED environment variable superseded by FLAGD_CACHE. Set FLAGD_CACHE to disabled to sustain previous behavior.

Related Issues

Fixes #117

Notes

Follow-up Tasks

How to test

Copy link
Member

@beeme1mr beeme1mr left a comment

Choose a reason for hiding this comment

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

Please also update the readme.
https://github.com/open-feature/go-sdk-contrib/tree/main/providers/flagd

While you're there, you can go ahead and remove the experimental and alpha badges.

if maxCacheSizeS != "" {
maxCacheSizeFromEnv, err := strconv.Atoi(maxCacheSizeS)
if err != nil {
p.logger.Error(err, fmt.Sprintf("invalid env config for %s provided, using default value", flagdMaxCacheSizeEnvironmentVariableName))
Copy link
Contributor

Choose a reason for hiding this comment

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

Minor - should we state the default value in the log 🤔 ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea, done

Copy link
Contributor

@Kavindu-Dodan Kavindu-Dodan left a comment

Choose a reason for hiding this comment

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

LGTM - Addressing my comment is optional

@toddbaert toddbaert self-requested a review February 21, 2023 16:24
@skyerus
Copy link
Contributor Author

skyerus commented Feb 21, 2023

Please also update the readme. https://github.com/open-feature/go-sdk-contrib/tree/main/providers/flagd

While you're there, you can go ahead and remove the experimental and alpha badges.

Well spotted, done.

@beeme1mr beeme1mr merged commit 5ee1f2c into open-feature:main Feb 21, 2023
@github-actions github-actions bot mentioned this pull request Feb 21, 2023
@skyerus skyerus deleted the issue-117/align-env-vars branch February 22, 2023 09:26
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 this pull request may close these issues.

[flagd] Align environment variables with flagd provider specification
5 participants