Repository navigation
Conversation
Fyusel
force-pushed
the
onboard-telemetryrouter
branch
from
September 14, 2026 08:39
4d194f4 to
8af9e54
Compare
|
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. |
marceljk
reviewed
Sep 29, 2026
Comment on lines
+44
to
+60
| Short: "Updates a TelemetryRouter instance", | ||
| Long: "Updates a TelemetryRouter instance.", | ||
| Args: args.SingleArg(argInstanceID, utils.ValidateUUID), | ||
| Example: examples.Build( | ||
| examples.NewExample( | ||
| `Update the display name of the TelemetryRouter instance with ID "xxx"`, | ||
| "$ stackit beta telemetryrouter instance update xxx --display-name new-name"), | ||
| examples.NewExample( | ||
| `Update the description of the TelemetryRouter instance with ID "xxx"`, | ||
| "$ stackit beta telemetryrouter instance update xxx --description new-description"), | ||
| examples.NewExample( | ||
| `Replace the filter attributes of the TelemetryRouter instance with ID "xxx" so it only routes telemetry data for HTTP GET or HEAD requests, matched at the resource level`, | ||
| `$ stackit beta telemetryrouter instance update xxx --filter-attribute "key=http.method;level=resource;matcher==;values=GET,HEAD"`), | ||
| examples.NewExample( | ||
| `Replace the filter attributes of the TelemetryRouter instance with ID "xxx" with many filter attributes sourced from a JSON file (recommended over repeating --filter-attribute for more than a handful of attributes); `+ | ||
| `use "stackit beta telemetryrouter instance generate-filter --instance-id xxx" to generate a starting point from the instance's current filter`, | ||
| `$ stackit beta telemetryrouter instance update xxx --filter @./filter.json`), |
Contributor
There was a problem hiding this comment.
I didn't tested it yet, but the endpoint behind this command is a PUT endpoint. If it's also implement to behave like a PUT, it means that everything that isn't set by the user during the update, will be removed. This relates not only to the filter / filter-attribute but also display name and description.
I think this should be mentioned here, to make aware of the different behavior.
Contributor
There was a problem hiding this comment.
Or it could be even discussed, if we want to have here the update command. Because this behave different like our existing update commands
Contributor
There was a problem hiding this comment.
Tested it today and the PUT endpoint behaves partial as patch and put. The name remains unchanged if it's not set during the update. But the description and filter are removed if they are not part of the update
marceljk
reviewed
Sep 30, 2026
marceljk
reviewed
Sep 30, 2026
|
|
||
| func NewCmd(params *types.CmdParams) *cobra.Command { | ||
| cmd := &cobra.Command{ | ||
| Use: fmt.Sprintf("update %s", destinationIdArg), |
Contributor
There was a problem hiding this comment.
This update command has also a mixed behavior. If description is unset during the update it will be removed, but the name and config remain unchanged if they are not set within the update.
I assume the behavior of the API is in general, that for fields that are not required during creation, it behaves like a PUT, but for fields that are required during creation, it behaves like a PATCH during the update.
Contributor
There was a problem hiding this comment.
same applies for the access-token update command
rubenhoenle
reviewed
Oct 9, 2026
Fyusel
force-pushed
the
onboard-telemetryrouter
branch
5 times, most recently
from
October 19, 2026 12:02
06b8a20 to
786f253
Compare
STACKITCLI-382 Signed-off-by: Alexander Dahmen <alexander.dahmen@inovex.de>
Fyusel
force-pushed
the
onboard-telemetryrouter
branch
from
October 19, 2026 12:39
786f253 to
3cc7194
Compare
Merging this branch changes the coverage (1 decrease, 20 increase)
Coverage detailsCompared to commit 538e0ac (run #39250588989)
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:
|
marceljk
reviewed
Oct 19, 2026
| type inputModel struct { | ||
| *globalflags.GlobalFlagModel | ||
| InstanceId string | ||
| Payload *telemetryrouter.CreateDestinationPayload |
Contributor
There was a problem hiding this comment.
I would suggest to not set the payload as pointer, because it's required and it safe potentially nil pointer
Suggested change
| Payload *telemetryrouter.CreateDestinationPayload | |
| Payload telemetryrouter.CreateDestinationPayload |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
STACKITCLI-382
Description
Checklist
make fmtmake generate-docs(will be checked by CI)make test(will be checked by CI)make lint(will be checked by CI)