-
Notifications
You must be signed in to change notification settings - Fork 1
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: Pick from list of real variations #420
Merged
mike-zorn
merged 29 commits into
main
from
mikezorn/sc-254693/validate-overrides-against-what-s-in-the
Sep 6, 2024
Merged
feat: Pick from list of real variations #420
mike-zorn
merged 29 commits into
main
from
mikezorn/sc-254693/validate-overrides-against-what-s-in-the
Sep 6, 2024
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
It will proxy the request to real launchdarkly, applying the access token.
Add source environment key to the UI
It was a bad idea. No auth + prod credz = security nightmares
* Do a whole lot of stuff * PR feedback
The fetchFlagState is only called from within the Project, so the tests were extraneous: tests of other public methods already covered the function.
Equivalent to UpdateProject wo/items. This fixed an issue where we didn't do the same thing when updating vs. syncing
Wanted better cohesion between state declarations and the data fetching.
cdelst
reviewed
Sep 6, 2024
@@ -70,7 +72,16 @@ func (s Sqlite) UpdateProject(ctx context.Context, project model.Project) (bool, | |||
return false, errors.Wrap(err, "unable to marshal flags state when updating project") | |||
} | |||
|
|||
result, err := s.database.ExecContext(ctx, ` | |||
tx, err := s.database.BeginTx(ctx, nil) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sqllite has transactions???
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah. SQLite is kinda incredible. It's one of the best tested and widely used pieces of software you'll find.
cdelst
approved these changes
Sep 6, 2024
mike-zorn
deleted the
mikezorn/sc-254693/validate-overrides-against-what-s-in-the
branch
September 6, 2024 16:55
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This adds better UX for multivariate flags. Previously, you just had the ability to edit the raw json values. Now you can pick from the available variations already defined in the LD service or you can create your own "local override". This is implemented by fetching all flags from the flag list API and writing the variations to the sqlite db. The data written to SQLite is then used to serve subsequent requests. I took this approach because it means that the pulled flag state should be in-sync with the available variations and it enables offline work. It does mean that add project & sync project operations take significantly longer in projects with large amounts of flags.
Some other changes are along for the ride because this branch was longer lived than ideal.