-
Notifications
You must be signed in to change notification settings - Fork 18
feat: openfeature pull command
#147
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: openfeature pull command
#147
Conversation
beeme1mr
left a comment
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.
I'd like to test this end-to-end in a bit but I wanted to submit my initial feedback. Overall, it looks great. Nice job!
7fc21f4 to
fd302ab
Compare
beeme1mr
left a comment
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.
Hey @kriscoleman, it's very close but the generated flags.json is invalid because the flag type is "int" instead of "integer".
You can reproduce this by running:
- go run cmd/openfeature/main.go init
- go run cmd/openfeature/main.go pull --flag-source-url https://raw.githubusercontent.com/kriscoleman/openfeature-cli/fd302ab33bc04ef4f7914455b7f4ee6ca0c8d2e9/sample/sample_source_flags.json
- go run cmd/openfeature/main.go generate react
The issue affects "flag3" in the manifest.
kriscoleman
left a comment
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.
just going back through again and I noticed that the integer flag in
flags.jsonit throwing an "incorrect type" error (expectingboolean), but i can manually correct it to `integer and it's all good.my intial thought would be changing the
IntTypereturn to "integer" instead of the "int" would make sure that the logic holds to thejson-schemavalidation and fix that but that could be too narrow of a focus on my part.
thank you! btw, what extension are you using to show that validation in your IDE? I love that
from what i can tell everything is still 👍🏽, but in case it caused any issues i missed wanted to atleast mention it here. 😄
![]()
kriscoleman
left a comment
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.
fd302ab to
50c5c6e
Compare
|
I approved but please consider #147 (comment) before merging. |
…pull command Signed-off-by: Jason Salaber <jcsalaber@hotmail.com> feat: added prompts for default values if not defined Signed-off-by: Jason Salaber <jcsalaber@hotmail.com> chore: conditionally add auth header if auth token is not empty Signed-off-by: Jason Salaber <jcsalaber@hotmail.com> chore: added tests for pull command Signed-off-by: Jason Salaber <jcsalaber@hotmail.com> feat: added ability to pull from local file for source flags Signed-off-by: Jason Salaber <jcsalaber@hotmail.com> chore: refactor of pull command Signed-off-by: Kris Coleman <kriscodeman@gmail.com>
50c5c6e to
dc52554
Compare
thanks, good call. I added in some logic to handle the prompt (but it also checks the override flag, if true, it just overrides) |
it this one called error lens! I’ve had it for so long I almost was like that is just what vs code does 😅. it has helped me so much in times of finding an error since it was more prominent! |
|
Thanks @kriscoleman, looks good. I'll leave it open for more feedback until tomorrow. FYI @cupofcat @anghelflorinm |
Consolidates the conditional logic for creating or updating the .openfeature.yaml file. This change improves readability and reduces code duplication by using a flag to determine whether the config file should be written. Signed-off-by: Kris Coleman <kriscodeman@gmail.com>
Refactors remote flag loading to handle errors more gracefully. This change ensures that the function returns nil immediately upon encountering an error, preventing potential issues with uninitialized flagsets. Additionally, initializes the flagset object only when standard manifest parsing is attempted. It then returns the flagset with source flags upon successful parsing. Signed-off-by: Kris Coleman <kriscodeman@gmail.com>
c70849c to
2cc9cf3
Compare
|
I'll give folks a day or two to finish reviews before merging. Thanks for getting this across the finish line @kriscoleman! |
Refactors the prompt logic for integer and float default values to use a new generic function. This improves code reusability and reduces duplication by centralizing the input validation logic. Signed-off-by: Kris Coleman <kriscodeman@gmail.com>
Moves the prompt definition outside the switch statement to avoid repetition and improves code readability. Signed-off-by: Kris Coleman <kriscodeman@gmail.com>
Refactors the init command to improve readability and maintainability. Moves logic for manifest creation and config file handling into separate functions for better organization. Introduces a confirmOverride function to reduce duplication and improve user experience when overriding existing files. Signed-off-by: Kris Coleman <kriscodeman@gmail.com>
This commit improves error handling in the init command. It addresses the issue where errors during confirmation prompts were not being properly handled, leading to unexpected behavior. Now, the application gracefully handles errors that may arise during the confirmation prompt, providing a more robust user experience. Signed-off-by: Kris Coleman <kriscodeman@gmail.com>
Refactors the way the default configuration file is generated. Instead of using string concatenation, it uses a template to generate the configuration file. This makes the configuration file more readable and easier to maintain and allows to handle conditional content. Signed-off-by: Kris Coleman <kriscodeman@gmail.com>
Adds the ability to marshal a Flagset into JSON format, making it compatible with the expected manifest structure. Also updates the write function to use the new marshalling logic to properly format the flags in the manifest file. Signed-off-by: Kris Coleman <kriscodeman@gmail.com>
Refactors manifest creation and writing logic into separate, reusable functions. This improves code readability and reduces duplication by extracting the manifest creation and writing processes into dedicated functions. This allows for more consistent handling of manifest operations across different parts of the codebase. Signed-off-by: Kris Coleman <kriscodeman@gmail.com>
Uses the abstract afero filesystem to read the flags file. This change improves testability and allows for easier manipulation of the filesystem in different environments. Signed-off-by: Kris Coleman <kriscodeman@gmail.com>
Ensures that the manifest file is written to disk atomically. This prevents potential data corruption or incomplete writes if the process is interrupted during the write operation. This implementation writes the manifest to a temporary file, then renames it to the target path, guaranteeing atomicity. Signed-off-by: Kris Coleman <kriscodeman@gmail.com>
Moves the promptWithValidation and promptForDefaultValue functions to the bottom of the file. This improves code organization and readability by grouping related functionality together. Signed-off-by: Kris Coleman <kriscodeman@gmail.com>
Fixes a bug where flag default values were not being correctly handled when prompting the user for input. This commit updates the code to use a pointer to the flag, ensuring that the default value is correctly assigned after prompting. Signed-off-by: Kris Coleman <kriscodeman@gmail.com>
Updates the error handling when unmarshaling JSON to provide a more accurate error message. Refactors the flagset writing process to directly process flag data, avoiding intermediate marshaling and unmarshaling steps. This simplifies the code and improves efficiency. Signed-off-by: Kris Coleman <kriscodeman@gmail.com>
2cc9cf3 to
22efb55
Compare
Refactors the flag source URL handling to use the `net/url` package. This change improves the robustness and clarity of the code by utilizing the `net/url` package to parse and validate the flag source URL. It now supports `file`, `http`, and `https` schemes. Signed-off-by: Kris Coleman <kriscodeman@gmail.com>
|
Thanks! Let's try and merge this tomorrow. |
Ensures that errors during temporary file removal are handled gracefully, preventing potential resource leaks or incomplete operations. It prioritizes the original error during manifest writing/renaming and logs the removal error to avoid masking the initial issue. Signed-off-by: Kris Coleman <kriscodeman@gmail.com>
Signed-off-by: Kris Coleman <kriscodeman@gmail.com>
4e3e492 to
7065148
Compare



Summary
This PR introduces a new
pullcommand to the OpenFeature CLI that enables fetching flag configurations from remote sources. Key changes include:pullcommand: Fetches flag manifests from remote HTTP/HTTPS URLs or local file pathsinitcommand: Now supports--flag-source-urlparameter to configure remote flag sources in.openfeature.yaml.openfeature.yamlconfig files to store flag source URLs and authentication tokensRelated Issues
#3
Architecture Changes
Loadfunctionality from flagset package to avoid circular dependenciesLoadFromSourceFlagsfunction: Handles type normalization for different flag type representations (e.g.,Boolean,bool,boolean)internal/manifest/pull.gofor better code organizationTesting
h2non/gockfor reliable remote source testingFuture Enhancements
Usage Example