-
Notifications
You must be signed in to change notification settings - Fork 26
Onboard Intake (1/3): introduce intake runner command #952
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
base: main
Are you sure you want to change the base?
Conversation
|
Sample CLI usage: |
732eff1 to
669dbb9
Compare
|
PS: notice that I messed up the branch by mistake, I had to cherry-pick until 7868e56 (last commit reviewed) and force push. |
|
Rest looks good to me. :) |
|
This PR was marked as stale after 7 days of inactivity and will be closed after another 7 days of further inactivity. If this PR should be kept open, just add a comment, remove the stale label or push new commits to it. |
|
keep PR open |
rubenhoenle
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please check the failing CI pipeline
# Conflicts: # go.mod # go.sum
| @@ -0,0 +1,162 @@ | |||
| package wait | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will be removed once the new SDK is published.
|
This PR was marked as stale after 7 days of inactivity and will be closed after another 7 days of further inactivity. If this PR should be kept open, just add a comment, remove the stale label or push new commits to it. |
|
This PR was marked as stale after 7 days of inactivity and will be closed after another 7 days of further inactivity. If this PR should be kept open, just add a comment, remove the stale label or push new commits to it. |
Co-authored-by: Ruben Hönle <[email protected]>
Co-authored-by: Ruben Hönle <[email protected]>
Co-authored-by: Ruben Hönle <[email protected]>
Co-authored-by: Ruben Hönle <[email protected]>
Co-authored-by: Ruben Hönle <[email protected]>
Co-authored-by: Ruben Hönle <[email protected]>
Co-authored-by: Ruben Hönle <[email protected]>
| } | ||
|
|
||
| func outputResult(p *print.Printer, outputFormat, projectLabel string, resp *intake.IntakeRunnerResponse) error { | ||
| switch outputFormat { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use p.OutputResult() instead. This saves yourself all the boilerplate code for the JSON/YAML output.
See the refactoring PR for reference: #1030
So your outputResult func here would look like this instead:
func outputResult(p *print.Printer, outputFormat, projectLabel string, resp *intake.IntakeRunnerResponse) error {
return p.OutputResult(outputFormat, resp, func() error {
if resp == nil {
p.Outputf("Created Intake Runner for project %q, but no runner ID was returned.\n", projectLabel)
return nil
}
p.Outputf("Created Intake Runner for project %q. Runner ID: %s\n", projectLabel, utils.PtrString(resp.Id))
return nil
}
}Please also apply this to all other subcommands 😅
|
|
||
| for _, tt := range tests { | ||
| t.Run(tt.description, func(t *testing.T) { | ||
| p := print.NewPrinter() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use the new testutils.TestParseInput func here to save us the boilerplate code.
| testutils.TestParseInput(t, NewCmd, parseInput, tt.expectedModel, tt.argValues, tt.flagValues, tt.isValid) |
see also the PR #1033 for reference
|
|
||
| for _, tt := range tests { | ||
| t.Run(tt.description, func(t *testing.T) { | ||
| p := print.NewPrinter() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use the new testutils.TestParseInput func here to save us the boilerplate code.
| testutils.TestParseInput(t, NewCmd, parseInput, tt.expectedModel, tt.argValues, tt.flagValues, tt.isValid) |
see also the PR #1033 for reference
|
|
||
| for _, tt := range tests { | ||
| t.Run(tt.description, func(t *testing.T) { | ||
| p := print.NewPrinter() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use the new testutils.TestParseInput func here to save us the boilerplate code.
| testutils.TestParseInput(t, NewCmd, parseInput, tt.expectedModel, tt.argValues, tt.flagValues, tt.isValid) |
see also the PR #1033 for reference
|
|
||
| for _, tt := range tests { | ||
| t.Run(tt.description, func(t *testing.T) { | ||
| p := print.NewPrinter() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use the new testutils.TestParseInput func here to save us the boilerplate code.
| testutils.TestParseInput(t, NewCmd, parseInput, tt.expectedModel, tt.argValues, tt.flagValues, tt.isValid) |
see also the PR #1033 for reference
|
|
||
| for _, tt := range tests { | ||
| t.Run(tt.description, func(t *testing.T) { | ||
| p := print.NewPrinter() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use the new testutils.TestParseInput func here to save us the boilerplate code.
| testutils.TestParseInput(t, NewCmd, parseInput, tt.expectedModel, tt.argValues, tt.flagValues, tt.isValid) |
see also the PR #1033 for reference
| ) | ||
|
|
||
| // ConfigureClient creates and configures a new Intake API client | ||
| func ConfigureClient(p *print.Printer, cliVersion string) (*intake.APIClient, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use our new genericclient.ConfigureClientGeneric util func here to save the boilerplate code, see the SKE example below how to use it:
stackit-cli/internal/pkg/services/ske/client/client.go
Lines 12 to 14 in d68210b
| func ConfigureClient(p *print.Printer, cliVersion string) (*ske.APIClient, error) { | |
| return genericclient.ConfigureClientGeneric(p, cliVersion, viper.GetString(config.SKECustomEndpointKey), false, genericclient.CreateApiClient[*ske.APIClient](ske.NewAPIClient)) | |
| } |
Refactoring PR for reference: #1058
| if model.Async { | ||
| operationState = "Triggered deletion of" | ||
| } | ||
| p.Printer.Info("%s stackit Intake Runner %s \n", operationState, model.RunnerId) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| p.Printer.Info("%s stackit Intake Runner %s \n", operationState, model.RunnerId) | |
| p.Printer.Outputf("%s stackit Intake Runner %s \n", operationState, model.RunnerId) |
p.Printer.Info prints to stderr, while p.Printer.Outputf prints to stdout. I guess you want to print to stdout here.
Description
This PR onboards the new STACKIT Intake (ticket) service into the CLI.
Intake is composed of three components:
This PR contains the Intake Runners part only to make a quicker and less overwhelming review. The remaining code (2/3) can be submitted as part of this PR once the first batch has been reviewed/approved or we can open a different PR for it (as the maintainer please).
The commands are designed to work as follows:
Most customers will use
intakeshence why we decided to go with this command structure. The folder structure will look like the following due to the hierachy desired:Checklist
make fmtmake generate-docs(will be checked by CI)make test(will be checked by CI)make lint(will be checked by CI)