renovatebot / renovate

Home of the Renovate CLI: Cross-platform Dependency Automation by Mend.io
https://mend.io/renovate
GNU Affero General Public License v3.0
17.66k stars 2.33k forks source link

Convert experimental env into globalOnly `experimentalFlags` #27879

Open rarkins opened 8 months ago

rarkins commented 8 months ago

Describe the proposed change(s).

Remove all RENOVATE_X_ logic in code, convert instead to a new admin option experimentalFlags. Convert docs accordingly (all should be documented and it enforced with type checking).

RahulGautamSingh commented 8 months ago

There are some variables in this list which don't have the prefix RENOVATE_X, should they be included in this refactor too? eg., https://docs.renovatebot.com/self-hosted-experimental/#otel_exporter_otlp_endpoint

RahulGautamSingh commented 8 months ago

Do you mean we store all these variables and values in experimentalFlags object? ie., old

# .env
RENOVATE_X_HARD_EXIT=false

new

// config.js
"exprimentalFlags": {
  "HARD_EXIT": false
}
rarkins commented 8 months ago

I was going to keep them as an array. If they need a value then it could be like abc=10. One like this could be turned around such as "softExit" or "exitCodeZero"

RahulGautamSingh commented 8 months ago

There are some options such as RENOVATE_X_IGNORE_RE2 which are used before GlobalConfig is initialized, what should be do about them?

rarkins commented 8 months ago

How many of those are there? I guess we need to keep them

RahulGautamSingh commented 8 months ago

Two with the prefix RENOVATE_X:

RENOVATE_X_HARD_EXIT https://github.com/renovatebot/renovate/blob/bf91e94b4b95a2a9274319aeaae8e03f05605258/lib/renovate.ts#L22

RENOVATE_X_IGNORE_RE2 https://github.com/renovatebot/renovate/blob/bf91e94b4b95a2a9274319aeaae8e03f05605258/lib/util/regex.ts#L18

rarkins commented 8 months ago

Ok keep them as is for now

RahulGautamSingh commented 7 months ago

I think we are clear on the requirements. Here's the implementation plan I came up with:

First make necessary changes in initial PR (ideally also convert 2,3 env vars to flags as examples). Then in many follow up PRs convert all the env vars to flags.

For this it would be best to create a new branch from the main branch, where all the initial and follow up PRs could be merged, before finally merging that one into main again. I am not sure about this btw, since these are experimental features doing all in one PR seems fine to me as well. But it should be easy in a multiple PRs.

Changes to be included in the initial PR:

I've put together this PR as a starting point. I've tried to implement the necessary logic to handle the exeperimental flags. I have also converted 2 env vars to flags in this PR as examples.

More details in the PR description.

rarkins commented 7 months ago

@viceice @secustor what if we drop the "experimental" naming and instead use "globalFlags" or "adminFlags"? We can always flag individual flags as experimental if we want to consider removing them.

Part of using flags here was to reduce the volume of unique config options when many of them are simply boolean and need little more than a one-sentence description.

secustor commented 7 months ago

IMO it make sense to have experimental in the naming. We communicate that these features are unstable and subject to change without looking at the docs. Further that extra scrutiny is expected as they are not part of the normal configuration.

If that is not the intend we can migrate all flags to config options with the experimental setting. 🤷

rarkins commented 7 months ago

ok

RahulGautamSingh commented 6 months ago

Except the following experimental env vars all the others have either been converted to their own config option, or to experimental flags. Some of the vars have also been removed.

Env Var Remarks
OTEL_EXPORTER_OTLP_ENDPOINT Used before global config is initialized
RENOVATE_X_HARD_EXIT For this to work, the return type of instrument will have to be changed which is not desirable I think hardExit
RENOVATE_X_IGNORE_RE2 Regex is used at one too many places to be sure, if it will be called before global config is initialized
RENOVATE_X_SUPPRESS_PRE_COMMIT_WARNING Used before global config is initialized
RENOVATE_X_REBASE_PAGINATION_LINKS Used before global config is initialized
RENOVATE_X_PAGINATE_ALL Used before global config is initialized
rofreytag commented 3 days ago

Hey there, related: RENOVATE_X_GITHUB_HOST_RULES was added in #25361 but there is no documentation for it. Also I would like to opt-in to using it with the hosted-by-mend variant, and I have no idea wether I can set this at all, so please consider this use case as well

rarkins commented 3 days ago

No. The main reason this feature was made opt-in was because it doesn't work with app tokens