Open
Conversation
- Added 'set' subcommand to the config command suite. - Implemented dynamic path resolution using constants.DefaultConfigPath to support Linux, macOS, and Windows. - Added logic to handle missing configuration files by creating them with 0644 permissions. - Integrated yaml.v3 for marshaling and unmarshaling config data. - Implemented project-specific error handling using output.Fatalf to comply with maintainer guidelines. - Isolated os.ErrNotExist from fatal I/O errors to prevent silent data loss. Issue: kitops-ml#419" Signed-off-by: Omar Bassem <omarbassem889@gmail.com>
- Extracted core file I/O logic into 'setConfig' function in config.go to separate CLI wiring from business logic. - Resturcutred error-handling worlflow to remove duplicate code, adhering to DRY principle. Issue: kitops-ml#419 Signed-off-by: Omar Bassem <omarbassem889@gmail.com>
Implement the `config get <key>` command to allow users to retrieve and print specific configuration values directly from the CLI. Refactor the underlying configuration logic by extracting YAML parsing and file reading into a shared helper function. This keeps the file I/O DRY and ensures that missing or unreadable file errors are handled safely and consistently across both the `set` and `get` operations. Issue: kitops-ml#419 Signed-off-by: Omar Bassem <omarbassem889@gmail.com>
Returning a closure from a function is unnecessary in this case, as there are no dependencies being injected to the wrapper function. Make functions private for now as they aren't being called outside the package. Issue: kitops-ml#419 Signed-off-by: Omar Bassem <omarbassem889@gmail.com>
- CLI output is explicitly sorted alphabetically to guarantee stable output and prevent Go's randomized map iteration from confusing the user. - Change loadConfigFile to be more idiomatic by returning a map rather than accepting a map as an argument. - Safely handle uninitialized environments by exiting quietly if config.yaml does not exist. Issue: kitops-ml#419 Signed-off-by: Omar Bassem <omarbassem889@gmail.com>
- Wipe all key-value pairs in the config.yaml file by overwriting it with an empty map, rather than deleting the file itself. - Add an interactive warning prompt before execution to ensure the user explicitly approves the destructive action. - Create pathHelper() to adhere to the DRY principle by returning both the Default and ConfigYaml paths. Issue: kitops-ml#419 Signed-off-by: Omar Bassem <omarbassem889@gmail.com>
Follow idiomatic Go practices by consolidating errors to their scope and removing unnecessary prefixes Issue: kitops-ml#419 Signed-off-by: Omar Bassem <omarbassem889@gmail.com>
- Use a struct to hold the global flags statically to provide type safety and reject invalid keys during configuration updates. - Format key-value pairs using JSON pretty-printing for improved terminal readability. Issue: kitops-ml#419 Signed-off-by: Omar Bassem <omarbassem889@gmail.com>
- Make the list command recognize when the config file does not exist and print an appropriate error message. - Make the set command ignore the absence of the config file and create a new file to store the key-value pair. Issue: kitops-ml#419 Signed-off-by: Omar Bassem <omarbassem889@gmail.com>
- Establish path precedence for configHome: CLI Flag > KITOPS_HOME Env Var > Config File > Defaults. - Establish setting precedence for log-level, progress, and verbosity: CLI Flags > Config File > Defaults. - Resolve config file load sequence to ensure the target directory is determined before reading the YAML file. - Implement graceful error handling to safely fall back to default values if the config file contains invalid strings. Issue: kitops-ml#419 Signed-off-by: Omar Bassem <omarbassem889@gmail.com>
Author
|
@amisevsk Appreciate the feedback. |
amisevsk
requested changes
Mar 19, 2026
…te errors -Remove `configHome` from the `Config` struct and related switch statements to eliminate a circular dependency. The CLI now strictly determines the configuration path via ephemeral flags, persistent environment variables, or hardcoded defaults before attempting file I/O. -Strip fallback logic from `root.go` (`PersistentPreRunE`) that incorrectly attempted to parse the storage directory from the file itself. -Update `saveConfigFile` to utilize `os.MkdirAll`, ensuring parent directories are safely generated before writing `config.yaml`. This prevents fatal "no such file or directory" panics on fresh installations. Issue: kitops-ml#419 Signed-off-by: Omar Bassem <omarbassem889@gmail.com>
- Move `output.SetOut` and `output.SetErr` to the very beginning of the `PersistentPreRunE` function. - This guarantees that Cobra's stream management is set up before any file system operations occur, ensuring that early initialization errors (like failing to locate the config directory) are correctly routed to the custom logger and can be reliably captured by automated tests. Issue: kitops-ml#419 Signed-off-by: Omar Bassem <omarbassem889@gmail.com>
Author
|
@amisevsk Would you kindly check the recent changes ? |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Description
This PR introduces the new
configcommand and natively integrates aconfig.yamlfile into the root CLI options, allowing users to persist global settings without needing to pass flags on every run.Added Subcommands:
config list: Outputs the entire active configuration state in a formatted JSON structure. Evaluates the complete precedence hierarchy to show the exact settings the CLI is using.config set <key> <value>: Writes a new configuration value directly to theconfig.yamlfile to persist global settings across terminal sessions.config get <key>: Retrieves and prints the value of a single, specific configuration setting.config reset: Deletes theconfig.yamlfile, completely clearing any persisted user settings and instantly reverting the CLI back to its default values.Key Implementations:
PersistentPreRunEhook so thatlog-level,progress, andverbosityare properly initialized for all subcommands.configHome): CLI Flag (--config) > Environment Variable ($KITOPS_HOME) > Config File > Defaults.log-level, etc.): CLI Flags > Config File > Defaults.Linked issues
AI-Assisted Code