Skip to content

Merge typed role and rolegroup configs before converting to product-config maps #284

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

Closed
nightkr opened this issue Dec 16, 2021 · 4 comments

Comments

@nightkr
Copy link
Member

nightkr commented Dec 16, 2021

Currently, overriding gets pretty tricky, since reasonable-looking code like this:

#[derive(Deserialize)]
struct FooConfig {
    #[serde(default = "FooConfig::default_port")]
    port: u16,
}

impl FooConfig {
    fn default_port() -> u16 {
        9191
    }
}

is subtly wrong, since the rolegroup's default port will override an explicitly specified role-level port. #282 made this somewhat worse, since it now triggers for all rolegroups (whereas previously it would only trigger for rolegroups that specified a config: field), but the issue was still there before.

@maltesander maltesander linked a pull request Aug 2, 2022 that will close this issue
5 tasks
@lfrancke lfrancke moved this to Development: Waiting for Review in Stackable Engineering Aug 12, 2022
@lfrancke lfrancke moved this from Development: Waiting for Review to Development: Track in Stackable Engineering Aug 23, 2022
@lfrancke
Copy link
Member

lfrancke commented Sep 7, 2022

@teozkr could you please add a link / quick explanation what this is blocked on?
Something in schemars if I'm correct

@lfrancke
Copy link
Member

@teozkr ping

@nightkr
Copy link
Member Author

nightkr commented Sep 13, 2022

Ah sorry, @lfrancke. This is blocked on GREsau/schemars#167, which is required in order to set up the correct type bounds in the generated schemas.

@lfrancke
Copy link
Member

If we don't get any feedback here, is there anything we can reasonably do?
e.g. ping one last time and let him know that we'll get started on our own PR starting from his draft?

@lfrancke lfrancke moved this from Development: Track to Development: Waiting for in Stackable Engineering Oct 5, 2022
@maltesander maltesander removed their assignment Oct 7, 2022
@lfrancke lfrancke moved this from Development: Waiting for to Ready for Development in Stackable Engineering Oct 7, 2022
@nightkr nightkr moved this from Ready for Development to Development: In Progress in Stackable Engineering Oct 13, 2022
@nightkr nightkr moved this from Development: In Progress to Development: Waiting for Review in Stackable Engineering Oct 13, 2022
@nightkr nightkr closed this as completed Oct 17, 2022
@nightkr nightkr moved this from Development: Waiting for Review to Development: Done in Stackable Engineering Oct 17, 2022
@lfrancke lfrancke moved this from Development: Done to Done in Stackable Engineering Oct 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
3 participants