Add missing fields to network types #278
Merged
Conversation
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
2 tasks
internet-diglett
added a commit
to oxidecomputer/omicron
that referenced
this pull request
May 31, 2025
This PR aims to improve the usability of the switch port settings API by returning data of similar shape and content as what users provide when the create the switch port settings. Issues resolved: * Before this PR, you only need one call to create a complete switch port settings object, but you have to perform several lookups to get the complete switch port configuration. We were already returning the object ids of the nested objects, so it makes a lot of sense to just return the actual object (especially since the user supplied the entire object when they configured it) * The `HashMap<String_of_link_name, T>` pattern also did not give the expected behavior in our go client generation. Changing this to a `Vec<T_with_link_name_inside>` matches what the user provides during creation and also resolves this behavior. * Some of our `Vec<T>` fields can be empty but must be present. In the Go client, an empty array is omitted from the json object. We needed to add a `default` annotation to these fields so Serde handles this situation correctly. ## Related - oxidecomputer/oxide.go#278 - oxidecomputer/terraform-provider-oxide#426 Closes #5780
hawkw
pushed a commit
to oxidecomputer/omicron
that referenced
this pull request
Jun 2, 2025
This PR aims to improve the usability of the switch port settings API by returning data of similar shape and content as what users provide when the create the switch port settings. Issues resolved: * Before this PR, you only need one call to create a complete switch port settings object, but you have to perform several lookups to get the complete switch port configuration. We were already returning the object ids of the nested objects, so it makes a lot of sense to just return the actual object (especially since the user supplied the entire object when they configured it) * The `HashMap<String_of_link_name, T>` pattern also did not give the expected behavior in our go client generation. Changing this to a `Vec<T_with_link_name_inside>` matches what the user provides during creation and also resolves this behavior. * Some of our `Vec<T>` fields can be empty but must be present. In the Go client, an empty array is omitted from the json object. We needed to add a `default` annotation to these fields so Serde handles this situation correctly. ## Related - oxidecomputer/oxide.go#278 - oxidecomputer/terraform-provider-oxide#426 Closes #5780
sudomateo
reviewed
Jun 26, 2025
There was a problem hiding this comment.
Do I have it correct that the only changes you need for the related oxidecomputer/terraform-provider-oxide#426 are the exceptions for the 3 types added in internal/generate/exceptions.go or are you making use of any of the new code generated in oxide/paths.go?
|
I think there were a lot of things generated because of the change that was made to the API tags on omicron. That was handled and fixed in #298. This PR probably just needs to rebase/merge with main, and a whole bunch of paths should just go away. #298 is also based on a newer commit, so we definitely want to use the omicron version there. |
sudomateo
force-pushed
the
add-missing-fields-to-network-types
branch
from
79551d4 to
86e49ef
Compare
|
I squashed the commits and rebased from |
Added network types to the nullable exception. Co-authored-by: Levon Tarver <11586085+internet-diglett@users.noreply.github.com>
sudomateo
force-pushed
the
add-missing-fields-to-network-types
branch
from
86e49ef to
8c8ed85
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
No description provided.