Change env-to-ini prefix and remove custom prefix #464

Merged
pat-s merged 2 commits from refs/pull/464/head into main 2023-07-16 22:00:49 +00:00
pat-s commented 2023-07-14 20:59:35 +00:00 (Migrated from gitea.com)

Description of the change

Change env-to-ini prefix and remove custom prefix.
GITEA is the default prefix.

Benefits

Compatibility wit v1.20 (-p got removed)

Possible drawbacks

None

Additional information

See https://github.com/go-gitea/gitea/pull/25799

Tested with Gitea < 1.20 and >= 1.20

### Description of the change Change env-to-ini prefix and remove custom prefix. `GITEA` is the default prefix. ### Benefits Compatibility wit v1.20 (`-p` got removed) ### Possible drawbacks None ### Additional information See https://github.com/go-gitea/gitea/pull/25799 Tested with Gitea < 1.20 and >= 1.20
lunny (Migrated from gitea.com) reviewed 2023-07-15 06:15:51 +00:00
techknowlogick (Migrated from gitea.com) reviewed 2023-07-15 06:15:53 +00:00
justusbunsi (Migrated from gitea.com) reviewed 2023-07-15 06:15:54 +00:00
justusbunsi commented 2023-07-16 20:38:34 +00:00 (Migrated from gitea.com)

From a user pov this change is a breaking one. Chart users are able to add custom envs with the previously used prefix to inject them into the ini building. Those envs must be changed. There is no backwards compatibility. We should explicitly state this. The actual changes look good.

From a user pov this change is a breaking one. Chart users are able to add custom envs with the previously used prefix to inject them into the ini building. Those envs must be changed. There is no backwards compatibility. We should explicitly state this. The actual changes look good.
wxiaoguang commented 2023-07-17 04:56:02 +00:00 (Migrated from gitea.com)

I am not sure about what's this line is used for:

    env | (grep GITEA || [[ $? == 1 ]]) > /tmp/existing-envs

If you want to filter the config options, maybe it should be grep GITEA__ (double underline) ?

I am not sure about what's this line is used for: ``` env | (grep GITEA || [[ $? == 1 ]]) > /tmp/existing-envs ``` If you want to filter the config options, maybe it should be `grep GITEA__` (double underline) ?
justusbunsi commented 2023-07-17 05:00:47 +00:00 (Migrated from gitea.com)

@wxiaoguang

I am not sure about what's this line is used for:

   env | (grep GITEA || [[ $? == 1 ]]) > /tmp/existing-envs

If you want to filter the config options, maybe it should be grep GITEA__ (double underline) ?

Good point. The old value was somewhat unique amongst the envs. This line saves the environment variables that exists prior to the script execution. It is necessary to override provided the ini values in the correct order.

https://gitea.com/gitea/helm-chart#user-defined-environment-variables-in-app-ini

Priority (highest to lowest) for defining app.ini variables:

  1. Environment variables prefixed with GITEA
  2. Additional config sources
  3. Values defined in gitea.config
@wxiaoguang >I am not sure about what's this line is used for: > >``` > env | (grep GITEA || [[ $? == 1 ]]) > /tmp/existing-envs >``` > >If you want to filter the config options, maybe it should be `grep GITEA__` (double underline) ? Good point. The old value was somewhat unique amongst the envs. This line saves the environment variables that exists prior to the script execution. It is necessary to override provided the ini values in the correct order. https://gitea.com/gitea/helm-chart#user-defined-environment-variables-in-app-ini > Priority (highest to lowest) for defining app.ini variables: > > 1. Environment variables prefixed with GITEA > 2. Additional config sources > 3. Values defined in gitea.config
pat-s commented 2023-07-17 06:24:10 +00:00 (Migrated from gitea.com)

Added a comment to the script, thanks everyone!

Added a comment to the script, thanks everyone!
justusbunsi commented 2023-07-17 07:00:48 +00:00 (Migrated from gitea.com)

I've marked this PR breaking. For release notes.

I've marked this PR breaking. For release notes.
Sign in to join this conversation.
No description provided.