Fix CLI usage with no pre-set context/config #420
Conversation
tstirrat15
changed the title
internal/storage/config.go
Outdated
| _, err := os.Stat(filepath.Join(s.ConfigPath, configFileName)) | ||
| if err != nil { | ||
| if os.IsNotExist(err) { | ||
| return false, nil | ||
| } | ||
| return false, err | ||
| } | ||
| return true, nil |
There was a problem hiding this comment.
purely a style nit, but I'm used to seeing this written more like:
| _, err := os.Stat(filepath.Join(s.ConfigPath, configFileName)) | |
| if err != nil { | |
| if os.IsNotExist(err) { | |
| return false, nil | |
| } | |
| return false, err | |
| } | |
| return true, nil | |
| if _, err := os.Stat(filepath.Join(s.ConfigPath, configFileName)); errors.Is(err, fs.ErrNotExist) { | |
| } else if err != nil { | |
| } else { | |
| } |
also apparently IsNotExist is soft-deprecated in favor of errors.Is: https://github.com/golang/go/blob/master/src/os/error.go#L88-L89
| return false, err | ||
| } | ||
|
|
||
| for _, token := range secrets.Tokens { |
There was a problem hiding this comment.
this is fine but makes me think Tokens should just be a map[string]Token instead of []Token
Also maybe this would make sense as a method on SecretStore?
There was a problem hiding this comment.
Yeah, as I was going through this there were a bunch of things that seemed like they could be reorganized. I want to get the fix out and then loop back.
| return storage.Token{}, err | ||
| } | ||
| if !configExists { | ||
| return GetTokenWithCLIOverride(cmd, storage.Token{}) |
There was a problem hiding this comment.
shouldn't CLI override... override the context?
Co-authored-by: Evan Cordell <[email protected]>
|
Follow-on work around code organization can happen in the context of #421. |
Description
In #417 we introduced a bug where
zedbecame unusable if there wasn't previously a context set by the user. This wasn't intentional; it should be valid to invoke zed entirely from the command line in isolation. This PR captures that behavior in a unit test and fixes the behavior.Changes
Testing
See that tests go green. See that you can clear out your local context (or run from within a container):
/zed schema write ./blah.zed --endpoint 127.0.0.1:50051 --token supersecretdevkey --insecureand not receive an error.