refactor: Rewrite config parser to respect the order

[Xuanwo]

Signed-off-by: Xuanwo github@xuanwo.io

I hereby agree to the terms of the CLA available at: https://databend.rs/dev/policies/cla/

Summary

This PR will fix #4362

To make our config parsed in a more understandable way, I built the following two libs:

• serde-bridge: A bridge between different serde implementations.
• serfig: Layered configuration system built upon serde

Now, our config is parsed in the following ways:

• Load from files (powered by toml and so on)
• Load from env (powered by serde-env)
• Load from clap args

All of them are bridged by serde-bridge.

With this PR, we will:

• Avoid args conflict via the new style of args naming style
• Remove env-helper! as we are loading by envy

Our config will be more consistent and extensible.

Breaking Changes

args naming

• From access-key-id to storage-s3-access-key-id
• From region to storage-s3-region
• Remove load_from_args, load_from_env, load_from_file from Config

All storage service-related args will be prefixed by storage-<service>- instead.

env naming

• From S3_STORAGE_ACCESS_KEY_ID to STORAGE_S3_ACCESS_KEY_ID
• Every env will map to an arg directly.

Preview

Current Status

PR is ready for review now!

Works are listed here

• Make sure all test cases passed
• Merge with main to adapt to new changes
• Make a release for serde-bridge, serfig and serde-env.
• Update all docs related to this change.

Changelog

• Improvement

Related Issues

Fixes #4362

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment

Name	Status	Preview	Updated
databend	⬜️ Ignored (Inspect)		Apr 24, 2022 at 5:25AM (UTC)

[mergify]

Thanks for the contribution!
I have applied any labels matching special text in your PR Changelog.

Please review the labels and make any necessary changes.

[ZhiHanZ]

I prefer load order like:

1. from argument
2. from configfile
3. from environment variable

to treat environment variable as first-class citizen and respect the configuration from configfile, and it would also cause painful breaking change if config from configfile could overlap settings from environment variables

[Xuanwo]

I prefer load order like:

1. from argument
2. from configfile
3. from environment variable

to treat environment variable as first-class citizen and respect the configuration from configfile, and it would also cause painful breaking change if config from configfile could overlap settings from environment variables

Yep, this proposal does implement this order, next layer will overwrite the same value for the current layer. That means we will take env first, and overload by config and argument.

[Xuanwo]

envy is not suitable for databend as it can't deserialize value in nested structs. I build a new lib serde-env to fix it.

Now, our PR is buildable. I'm going to move to the next step: make PR ready for review 🚀

[Xuanwo]

Oh, I love deleting code (

[Xuanwo]

I added a test for override: https://github.com/datafuselabs/databend/pull/4971/files#diff-a8af230cc9f832a19df365f4ddd5cf49cc3bd752705c27445008c928d7f2041dR193-R295

@ZhiHanZ Please help check whether this behavior is expected.

[sundy-li]

/LGTM Tests should be fixed

[flaneur2020]

I guess we can consider a versioned config approach like: https://github.com/k3d-io/k3d/tree/main/pkg/config

on introducing incompatible config changes, we can bounce up the config version with a migration logic, like https://github.com/flaneur2020/k3d/blob/412950bd84bcee0050c42515ef19356689152328/pkg/config/v1alpha3/migrations.go#L41

thus the older configs can keep compatible until the annual deprecation cycle, allowing users to migrate the configs after reading our deprecation news but before the actual api got break.

[Xuanwo]

I guess we can consider a versioned config approach like: https://github.com/k3d-io/k3d/tree/main/pkg/config

I will add a version id first in this PR.

Tracked at #5022

[Xuanwo]

test_stateless_standalone_macos seems not very stable.

[Xuanwo]

ping @BohuTANG @flaneur2020 for another approve.

[BohuTANG]

lgtm