search

authzed / zed

Official command-line tool for managing SpiceDB
https://authzed.com/docs/reference/clients
Apache License 2.0
117 stars 26 forks source link

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

Open tstirrat15 opened 1 month ago

tstirrat15 commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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 https://github.com/authzed/zed/pull/417, which then introduced https://github.com/authzed/zed/issues/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.