Add k6 cloud project and k6 cloud project list commands#5650
Add k6 cloud project and k6 cloud project list commands#5650
k6 cloud project and k6 cloud project list commands#5650Conversation
k6 cloud project and k6 cloud project list commands
|
Adding you as a reviewer @AgnesToulet as you've been working actively on this recently 🙇🏻 |
8773e25 to
021ae2f
Compare
| return c.outputJSON(resp.Value) | ||
| } | ||
|
|
||
| stackHeader := fmt.Sprintf("Projects for stack %d:\n\n", cloudConfig.StackID.Int64) |
There was a problem hiding this comment.
I think if we have the StackURL field non-empty, we should use it instead of the stack ID (but it might be empty).
| return errors.New( | ||
| "no stack configured. Please run `k6 cloud login --stack <your-stack>` to set a default stack", | ||
| ) |
There was a problem hiding this comment.
Might be worth creating a constant for this error, we'll probably use it a lot in other parts of the codebase in the future.
|
Moving it to the next milestone to unlock the ongoing release process. |
b1a3519 to
54a482d
Compare
|
@oleiade it seems that this needs some updates 😬 |
| t.Parallel() | ||
|
|
||
| srv := mockProjectListServer(t) | ||
| defer srv.Close() |
There was a problem hiding this comment.
I think this should be within a t.Cleanup(...), and perhaps, that could even be called inside mockProjectListServer, as it already receives t, and so this way no caller will ever forget to do so.
Wdyt?
| listCmd.SetUsageTemplate(defaultUsageTemplate) | ||
| cloudProjectCommand.AddCommand(listCmd) | ||
|
|
||
| cloudProjectCommand.SetUsageTemplate(`Usage: |
There was a problem hiding this comment.
I see the list command help is convoluted, would make sense to use something like this there as well? I'm wondering if we should even define this as a function/const to be reused more widely.
| if !checkIfMigrationCompleted(c.globalState) { | ||
| if err := migrateLegacyConfigFileIfAny(c.globalState); err != nil { | ||
| return err | ||
| } | ||
| } |
There was a problem hiding this comment.
Ideally, I'd prefer if we could do this in a single place. If not, it's okay.
I haven't digged into Cobra details since a while ago, so I forgot almost all my knowledge about it, but I was wondering if there's a way so any k6 cloud ... command executes this code, as a "pre-run" or something like that, but defined in a single place. Would that make sense? Not sure if possible, tho.
There was a problem hiding this comment.
I guess the same reasoning could apply to the lines below, again, if possible, as ideally I'd like to not end up with the same code in ~20 places if we start adding support for more project operations or support for other types of resources (e.g., load tests).
| if len(resp.Value) == 0 { | ||
| printToStdout(c.globalState, stackHeader+ | ||
| "No projects found.\n"+ | ||
| "To create a project, visit https://grafana.com/docs/grafana-cloud/testing/k6/projects/\n") |
There was a problem hiding this comment.
The URL seems broken, I think it should be either of:
|
|
||
| if !cloudConfig.StackID.Valid || cloudConfig.StackID.Int64 == 0 { | ||
| return errors.New( | ||
| "no stack configured. Please run `k6 cloud login --stack <your-stack>` to set a default stack", |
There was a problem hiding this comment.
| "no stack configured. Please run `k6 cloud login --stack <your-stack>` to set a default stack", | |
| "no stack configured. Please run `k6 cloud login` to set a default stack", |
I agree. We should just suggest k6 cloud login. However, we already have this logic for checking the user session. We should reuse that logic from a shared place so we don't reinvent the wheel here.
What?
This pull request introduces a new
cloud projectcommand group to the CLI, with ak6 cloud project listsubcommand allowing users to list their Grafana Cloud k6 projects. It adds the implementation, command wiring, output formatting (including JSON support), and comprehensive tests for the new functionality.Why?
As per #5644.
Checklist
make check) and all pass.Checklist: Documentation (only for k6 maintainers and if relevant)
Please do not merge this PR until the following items are filled out.
Related PR(s)/Issue(s)
#5644