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

Config system overhaul #24804

Open
silverwind opened this issue May 19, 2023 · 3 comments
Open

Config system overhaul #24804

silverwind opened this issue May 19, 2023 · 3 comments
Labels
type/proposal The new feature has not been accepted yet but needs to be discussed first.

Comments

@silverwind
Copy link
Member

silverwind commented May 19, 2023

Feature Description

Gitea's config system has various issues:

  1. Environment variables are not recognized, unless environment-to-ini is loaded, which currently only runs for docker deployments.
  2. There are two places to define a default value and it causes bugs, as seen in this PR.
  3. There is ambiguity between casing, e.g. Enabled in the config struct, vs ENABLED in the .Key definition.

I would propose treating environment variables as first-class, so the following steps could be taken:

  1. Move environment-to-ini into the main code base so environment variables are recognized at all times.
  2. Use https://github.com/kelseyhightower/envconfig to parse the environment to config structs. The structs should have original case and format of the variables, so a config variable remains greppable, so it should be config.APP_NAME instead of config.AppName.
  3. Have a separate mechanism that parses the ini and map values to the already initialized config struct, but not overwrite existing values because environment variables must have precedence over ini values.
@silverwind silverwind added type/proposal The new feature has not been accepted yet but needs to be discussed first. type/feature Completely new functionality. Can only be merged if feature freeze is not active. labels May 19, 2023
@lunny
Copy link
Member

lunny commented May 19, 2023

I will against to support these hundreds of envs in Gitea directly. It will become unmaintainable thing.

@lunny lunny removed the type/feature Completely new functionality. Can only be merged if feature freeze is not active. label May 19, 2023
@silverwind
Copy link
Member Author

silverwind commented May 19, 2023

I think at minimum, we want to merge environment-to-ini into the codebase. It's just not acceptable to require a config file in this day and age.

It should be possible to start gitea in systemd with a bunch of variables in the gitea.service file only. See the gitpod config which had to specifically work around this silly requirement for a app.ini.

@wxiaoguang
Copy link
Contributor

There are two places to define a default value and it causes bugs, as seen in this PR.

That's also related to the ini package's bug / quirk behavior. Sooner or later, the ini package should be written first before other config related refactoring.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/proposal The new feature has not been accepted yet but needs to be discussed first.
Projects
None yet
Development

No branches or pull requests

3 participants