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: migrate flagd startup argument to sources flag #427

Merged
merged 4 commits into from
Mar 31, 2023

Conversation

Kavindu-Dodan
Copy link
Contributor

@Kavindu-Dodan Kavindu-Dodan commented Mar 29, 2023

Prerequisite

Release of flagd 0.4.6

This PR

Fixes #389

This PR migrate flagd side-car startup arguments from uri (flagd start --uri <sync-source>) to sources (flagd start --sources <sync-source>).

More on the new sources based configuration options [1]

With this option, I have introduced the grpc sync option to OFO and other grpc sync-provider options.

follow-up

Discussion - type SourceConfig can be referred from OFO dependency and avoid duplication at flagd

[1] - https://github.com/open-feature/flagd/blob/main/docs/configuration/configuration.md

@Kavindu-Dodan Kavindu-Dodan requested a review from a team as a code owner March 29, 2023 19:07
@codecov
Copy link

codecov bot commented Mar 29, 2023

Codecov Report

Merging #427 (06eb625) into main (651c63c) will increase coverage by 1.36%.
The diff coverage is 59.84%.

@@            Coverage Diff             @@
##             main     #427      +/-   ##
==========================================
+ Coverage   77.76%   79.13%   +1.36%     
==========================================
  Files          21       21              
  Lines        1300     1323      +23     
==========================================
+ Hits         1011     1047      +36     
+ Misses        248      234      -14     
- Partials       41       42       +1     
Impacted Files Coverage Δ
...pis/core/v1alpha3/flagsourceconfiguration_types.go 100.00% <ø> (ø)
webhooks/pod_webhook.go 81.71% <55.65%> (+3.48%) ⬆️
...pis/core/v1alpha1/flagsourceconfiguration_types.go 98.78% <100.00%> (+0.01%) ⬆️
...ore/v1alpha3/flagsourceconfiguration_conversion.go 100.00% <100.00%> (ø)
Flag Coverage Δ
component-tests 78.76% <55.65%> (+2.92%) ⬆️
unit-tests 61.37% <9.44%> (-0.32%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

@Kavindu-Dodan Kavindu-Dodan changed the title feat: migrate to flagd startup argument to sources flag feat: migrate flagd startup argument to sources flag Mar 29, 2023
Copy link
Contributor

@james-milligan james-milligan left a comment

Choose a reason for hiding this comment

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

few small comments, but LGTM

Copy link
Member

@toddbaert toddbaert left a comment

Choose a reason for hiding this comment

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

Thanks for circling back to this after the related flagd changes. I tried to find something to be improved but I can't 😅 . LGTM.

@Kavindu-Dodan
Copy link
Contributor Author

Thanks for circling back to this after the related flagd changes. I tried to find something to be improved but I can't 😅 . LGTM.

Thank you :)

Signed-off-by: Kavindu Dodanduwa <kavindudodanduwa@gmail.com>
Signed-off-by: Kavindu Dodanduwa <kavindudodanduwa@gmail.com>
Signed-off-by: Kavindu Dodanduwa <kavindudodanduwa@gmail.com>
Signed-off-by: Kavindu Dodanduwa <kavindudodanduwa@gmail.com>
@Kavindu-Dodan Kavindu-Dodan merged commit 1c67f34 into open-feature:main Mar 31, 2023
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.

Use new --sources arg in flagd startup in pod webook
3 participants