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

chore: Rethink code flow for partial specification of CLI options #421

Open
tstirrat15 opened this issue Sep 19, 2024 · 2 comments
Open

Comments

@tstirrat15
Copy link
Contributor

Specifically:

  1. If a context is selected but an endpoint is provided, should it attempt to use the other values in the context for the other flags, or should that be treated as a new context?
  2. If a context is selected but other values are overridden (e.g. --insecure), should it use the other values available?

We could either treat it as "CLI overrides values otherwise provided by context" or "providing CLI values that would otherwise be in context prevents usage of context." The latter is easier to reason about from a code perspective, but the behavior may not be immediately apparent.

@tstirrat15
Copy link
Contributor Author

This was the context that led to #417:

$ zed version --no-verify-ca --log-level trace
2:32PM DBG configured logging async=false format=auto log_level=trace provider=zerolog
client: zed v0.21.1
2:33PM TRC token={"APIToken":"[...]","CACert":null,"Endpoint":"some.domain.here:443","Insecure":false,"Name":"thing","NoVerifyCA":false}

$ zed schema read --log-level trace --no-verify-ca --endpoint some.domain.here:443 --token some_token
2:43PM DBG configured logging async=false format=auto log_level=trace provider=zerolog
2:43PM TRC token={"APIToken":"[...]","CACert":null,"Endpoint":"some.domain.here:443","Insecure":null,"Name":"env","NoVerifyCA":null}

Someone was attempting to provide --no-verify-ca with their context and it wasn't being respected.

@vroldanbet
Copy link
Contributor

The behavior until now has been roughly CLI overrides values otherwise provided by context, except some flags were being ignored as input for the override, which led to opening #417, which then introduced #418.

Since that has been the contract in place, I think we should honor it. Now, is the current behavior "surprising", "unexpected" or difficult to reason about? I do understand the alternative scenario where the developer wants to get in control of the inputs, and does not expect the context to be used at all.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants