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(config): allow config dir to be passed as argument #1850

Merged
merged 1 commit into from
Nov 18, 2024

Conversation

sthaha
Copy link
Collaborator

@sthaha sthaha commented Nov 18, 2024

This commit now allows default hard-coded config directory to be passed as an argument. This allow quickly changing between different configurations to be stored separately and to switch between them (especially during development).

The commit also

  • simplifies global config initialization by ensuring it is initialised at the time kepler's main function is executed and fail with error that step fails.

  • It also cleans up use of config object to read CGroup info by creating a realSystem struct that handles this functionality.

Copy link
Contributor

github-actions bot commented Nov 18, 2024

🤖 SeineSailor

Here is a concise summary of the pull request changes:

Summary: This pull request introduces a new --config-dir flag, simplifies global config initialization, and refactors config object usage for CGroup info. It also updates various test files to accommodate these changes.

Key Modifications:

  1. Config Directory Flag: A new --config-dir flag allows passing a custom config directory, enabling faster configuration switching during development.
  2. Simplified Global Config Initialization: The newAppConfig() signature has been altered to include a BaseDir field, and config is now initialized with BaseDir from appConfig.
  3. Refactored Config Object Usage: The realSystem struct has been introduced to clean up config object usage for CGroup info.
  4. Test File Updates: Multiple test files have been updated to replace config.GetConfig() with config.Initialize() or config.Instance(), and the ensureConfigInitialized function has been removed.

Impact on Codebase:

  • Improved flexibility in configuration management
  • Simplified global config initialization
  • Enhanced code organization and readability

Suggestions for Improvement:

  • Consider adding documentation or comments to explain the purpose and usage of the new --config-dir flag.
  • Review the updated test files to ensure they cover all necessary scenarios and edge cases.
  • Verify that the realSystem struct is properly utilized throughout the codebase to maintain consistency.

@sthaha sthaha force-pushed the feat-config-dir-arg branch 3 times, most recently from 73f6320 to 5bd0400 Compare November 18, 2024 09:29
This commit now allows default hard-coded config directory to be passed
as an argument. This allow quickly changing between different
configurations to be stored separately and to switch between them
(especially during development).

The commit also
* simplifies global config initialization by ensuring  it is initialised
  at the time kepler's main function is executed and fail with error that
  step fails.

* It also cleans up use of config object to read CGroup info by creating
  a `realSystem` struct that handles this functionality.

Signed-off-by: Sunil Thaha <[email protected]>
@sthaha sthaha force-pushed the feat-config-dir-arg branch from 5bd0400 to dea318e Compare November 18, 2024 11:15
@sthaha sthaha requested a review from rootfs November 18, 2024 11:30
@rootfs rootfs merged commit 11711e7 into sustainable-computing-io:main Nov 18, 2024
20 checks passed
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.

2 participants