feat(cdn): add cdn client, config, list command #1100
Conversation
| return cmd | ||
| } | ||
|
|
||
| var sortByFlagOptions = []string{"id", "created", "updated", "origin-url", "status"} |
There was a problem hiding this comment.
Please directly use directly the API values: "id" "updatedAt" "createdAt" "originUrl" "status" "originUrlRelated"
This was wrong in the Jira story.
There was a problem hiding this comment.
Maybe there's also a enum generated in the SDK which holds these values. Then this would be the way to go because it will be kept up-to-date in the future without any efforts on our side.
There was a problem hiding this comment.
Searched for sortBy and originUrl etc. could not find an enum for this. The spec looks like it should generate an enum: https://github.com/stackitcloud/stackit-api-specifications/blob/83214868e6dbdc53d053cc07654542d8f34b5491/services/cdn/v1beta2/cdn.json#L1486
How could I start to investigate this further?
|
|
||
| func outputResult(p *print.Printer, outputFormat string, distributions []cdn.Distribution) error { | ||
| if distributions == nil { | ||
| distributions = make([]cdn.Distribution, 0) // otherwise prints null in json output |
There was a problem hiding this comment.
That's a good catch indeed! Any chance we can solve this in a central place for all commands?
There was a problem hiding this comment.
Nice idea, I'll investigate this a bit, don't know much about go reflection yet.
There was a problem hiding this comment.
Reflection is considered a no-no, see https://principles.schwarz/principles/engineering-principles/go/avoid-using-reflection
There was a problem hiding this comment.
With reflection I'd add this in Printer.OutputResult():
val := reflect.ValueOf(output)
if val.IsNil() {
// when passing a nil slice or map, JSON and YAML marshal to "null"
// so we need to create an empty slice or map to have "[]" or "{}" output instead
switch val.Kind() {
case reflect.Slice:
output = make([]any, 0)
case reflect.Map:
output = make(map[any]any)
default:
// do nothing
}
}The linked document suggests type switches, which would not work here.
Another solution without reflection, that comes to mind, would be to introduce specialized OutputResult functions, like OutputResultSlice/Map.
Co-authored-by: Ruben Hönle <[email protected]>
Co-authored-by: Ruben Hönle <[email protected]>
| var testProjectId = uuid.NewString() | ||
| var testClient = &cdn.APIClient{} | ||
| var testCtx = context.WithValue(context.Background(), "foo", "foo") | ||
| var testNextPageID = "next-page-id-123" |
There was a problem hiding this comment.
could be const I guess 😅 (yeah I know, nitpick)
There was a problem hiding this comment.
We need to take the address of testNextPageID in some tests to pass it to the generated API -> const does not work.
There was a problem hiding this comment.
nvm. just discovered utils.Ptr() for this.
| var testCtx = context.WithValue(context.Background(), "foo", "foo") | ||
| var testNextPageID = "next-page-id-123" | ||
| var testTime = time.Now() | ||
| var testID = "dist-1" |
There was a problem hiding this comment.
We also need the address of this one.
|
Btw, please always add the Jira issue ID to your PR description if possible :) |
| } | ||
|
|
||
| table := tables.NewTable() | ||
| table.SetHeader("ID", "REGIONS", "STATUS") |
There was a problem hiding this comment.
@rubenhoenle, when using the table output we only get these columns. When using the JSON format we would get all fields of cdn.Distribution. Should I introduce here custom struct that only contains ID, REGIONS and STATUS in JSON?
Merging this branch changes the coverage (1 decrease, 3 increase)
Coverage by fileChanged files (no unit tests)
Please note that the "Total", "Covered", and "Missed" counts above refer to code statements instead of lines of code. The value in brackets refers to the test coverage of that file in the old version of the code. Changed unit test files
|
Merging this branch changes the coverage (1 decrease, 3 increase)
Coverage by fileChanged files (no unit tests)
Please note that the "Total", "Covered", and "Missed" counts above refer to code statements instead of lines of code. The value in brackets refers to the test coverage of that file in the old version of the code. Changed unit test files
|
Description
Issue: STACKITCLI-70
relates to #1234
Checklist
make fmtmake generate-docs(will be checked by CI)make test(will be checked by CI)make lint(will be checked by CI)