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

add toml config option to cmd #172

Merged
merged 3 commits into from
Nov 15, 2024
Merged

Conversation

Yiling-J
Copy link
Member

@Yiling-J Yiling-J commented Nov 15, 2024

This PR introduces a --config option to all cmd commands, enabling configuration to be read from a TOML config file. The existing environment-based configuration still working and should not be affected by this.

Example Usage:

go run cmd/csghub-server/main.go start server --config local.toml
go run cmd/csghub-server/main.go deploy runner --config local.toml

An example configuration file has been added. To set up for local development, rename it to local.toml, place it in the project root, and customize as needed. While other names can be used, local.toml is added to .gitignore to prevent accidental commits.

Replace github.com/kelseyhightower/envconfig with github.com/sethvargo/go-envconfig because only sethvargo/go-envconfig supports overwrite mode. Note that the syntax of the config struct tags has changed because of this update.

Notes:

When both config file and environment variables are used, environment variables always take precedence, overwriting same config from the config file.

If any configurations are missing in the config file, default values are provided based on the default struct tag.

MR Summary:

The summary is added by @codegpt.

This Merge Request introduces a --config option for cmd commands, allowing configurations to be read from a TOML file, while maintaining compatibility with the existing environment variable-based configuration. It replaces github.com/kelseyhightower/envconfig with github.com/sethvargo/go-envconfig to support overwrite mode and changes the syntax for config struct tags. The MR ensures that environment variables take precedence over config file settings and provides default values for missing configurations. Additionally, it includes a test suite for the new configuration loading mechanism.

Key updates:

  1. Added support for TOML config files in cmd commands.
  2. Replaced envconfig package to support overwrite mode.
  3. Environment variables override TOML file configurations.
  4. Default values are used for missing configurations in the TOML file.
  5. Added tests for configuration loading.

@starship-github
Copy link

Linter Issue Report

During the code review, a list issues were found. These issues could affect the code quality, maintainability, and consistency. Below is the detailed Linter issue report:

common/config/config.go

Lint Issue: undefined: envconfig

  • Location: Line 237, Column 9
  • Code Context:
    err := envconfig.ProcessWith(context.Background(), &envconfig.Config{
  • Suggestion: It appears that the envconfig package is being used but not properly defined or imported in this file. Ensure that you have imported the envconfig package correctly at the top of your file. If the package is not installed, you may need to get it using go get github.com/sethvargo/go-envconfig or add it to your project's dependencies. After ensuring the package is available, import it with import "github.com/sethvargo/go-envconfig" at the beginning of your file.

Please make the suggested changes to improve the code quality.

@starship-github
Copy link

Possible Issues And Suggestions:

  • common/config/config_test.go
    • Comments:
      • The test case 'config file' assumes a 'test.toml' file exists, but there's no indication this file is created or exists in the test setup. This could lead to the test failing due to a missing file.

MR Evaluation:

This feature is still under test, evaluation are given by AI and might be inaccurate.

After evaluation, the code changes in the Merge Request get score: 100.

Tips

CodeReview Commands (invoked as MR or PR comments)

  • @codegpt /review to trigger an code review.
  • @codegpt /evaluate to trigger code evaluation process.
  • @codegpt /describe to regenerate the summary of the MR.
  • @codegpt /secscan to scan security vulnerabilities for the MR or the Repository.
  • @codegpt /help to get help.

CodeReview Discussion Chat

There are 2 ways to chat with Starship CodeReview:

  • Review comments: Directly reply to a review comment made by StarShip.
    Example:
    • @codegpt How to fix this bug?
  • Files and specific lines of code (under the "Files changed" tab):
    Tag @codegpt in a new review comment at the desired location with your query.
    Examples:
    • @codegpt generate unit testing code for this code snippet.

Note: Be mindful of the bot's finite context window.
It's strongly recommended to break down tasks such as reading entire modules into smaller chunks.
For a focused discussion, use review comments to chat about specific files and their changes, instead of using the MR/PR comments.

CodeReview Documentation and Community

  • Visit our Documentation
    for detailed information on how to use Starship CodeReview.

About Us:

Visit the OpenCSG StarShip website for the Dashboard and detailed information on CodeReview, CodeGen, and other StarShip modules.

@Yiling-J Yiling-J force-pushed the feature/toml_config branch from 2f184ba to cc4efc1 Compare November 15, 2024 01:30
Copy link
Collaborator

@ganisback ganisback left a comment

Choose a reason for hiding this comment

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

LGTM

@Yiling-J Yiling-J merged commit 6f9ebbf into OpenCSGs:main Nov 15, 2024
@starship-github
Copy link

The StarShip CodeReviewer was triggered but terminated because it encountered an issue: The MR state is not opened.

Tips

CodeReview Commands (invoked as MR or PR comments)

  • @codegpt /review to trigger an code review.
  • @codegpt /evaluate to trigger code evaluation process.
  • @codegpt /describe to regenerate the summary of the MR.
  • @codegpt /secscan to scan security vulnerabilities for the MR or the Repository.
  • @codegpt /help to get help.

CodeReview Discussion Chat

There are 2 ways to chat with Starship CodeReview:

  • Review comments: Directly reply to a review comment made by StarShip.
    Example:
    • @codegpt How to fix this bug?
  • Files and specific lines of code (under the "Files changed" tab):
    Tag @codegpt in a new review comment at the desired location with your query.
    Examples:
    • @codegpt generate unit testing code for this code snippet.

Note: Be mindful of the bot's finite context window.
It's strongly recommended to break down tasks such as reading entire modules into smaller chunks.
For a focused discussion, use review comments to chat about specific files and their changes, instead of using the MR/PR comments.

CodeReview Documentation and Community

  • Visit our Documentation
    for detailed information on how to use Starship CodeReview.

ganisback pushed a commit that referenced this pull request Dec 5, 2024
* Add toml config option to cmd

* update git ignore

* add config cmd

---------

Co-authored-by: yiling <yiling@yilingdeMacBook-Air.local>
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