refactor: helper function for getToken for cache reset + error handling#133
refactor: helper function for getToken for cache reset + error handling#133srikarmanikonda wants to merge 2 commits intodepot:mainfrom
Conversation
jacobwgillespie
left a comment
jacobwgillespie
left a comment
There was a problem hiding this comment.
Awesome, thank you for this! 🎉 I left a few comments below, let me know if anything isn't clear, etc.
| } | ||
| if token == "" { | ||
| return fmt.Errorf("missing API token, please run `depot login`") | ||
| token, err := getToken() |
There was a problem hiding this comment.
Looks like the build and bake commands are already using a helper function, you might be able to use the same one here as well:
token = helpers.ResolveToken(context.Background(), token)| var cwd string | ||
| if len(args) > 0 { | ||
| cwd, _ = filepath.Abs(args[0]) | ||
| cwd, err := filepath.Abs(args[0]) | ||
| if err != nil { | ||
| return fmt.Errorf("unable to determine absolute path: %w", err) |
There was a problem hiding this comment.
The logic from before isn't really easy to follow, sorry about that. What's happening is that cache reset can take either 0 or 1 arguments. If no arguments are specified, then the cwd variable will contain an empty string "".
Then if an empty string passed to helpers.ResolveProjectID(projectID, cwd) as the cwd, that function will actually resolve the current working directory.
So we do only want to try to resolve args[0] if len(args) > 0.
Maybe to make it clearer, we could rename the cwd variable here cwdOverride or projectDir or something that would make it more clear?
We could also move the resolution of cwd, _ := filepath.Abs(...) into the helper function itself, I imagine every command has that part duplicated currently.
| if projectID == "" { | ||
| return errors.Errorf("unknown project ID (run `depot init` or use --project or $DEPOT_PROJECT_ID)") | ||
| } |
There was a problem hiding this comment.
I think this line is duplicated:
| if projectID == "" { | |
| return errors.Errorf("unknown project ID (run `depot init` or use --project or $DEPOT_PROJECT_ID)") | |
| } |
|
Nice I think I got the Actions CI workflows to work for PRs as well |
2 participants
I included a quick helper function to simplify code reading for getting a token in cache reset + added some error messages for users to see if cache resets with no args