Skip to content
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

Setting optional attributes to their "empty" value #107

Closed
fbreckle opened this issue Oct 26, 2020 · 6 comments
Closed

Setting optional attributes to their "empty" value #107

fbreckle opened this issue Oct 26, 2020 · 6 comments

Comments

@fbreckle
Copy link

fbreckle commented Oct 26, 2020

Hi,

this is a generalization of tickets like #105 and #106 because it applies to way more attributes than mentioned in these issues.

The problem
Setting an optional attribute to its "empty" value (false, 0, a nil pointer, a nil interface value, and any empty array, slice, map, or string) will remove the attribute from the resulting JSON that is sent to netbox. This means that these values can not be set to their empty values via go-netbox.

The cause
This happens because in the swagger.json for optional attributes looks like this (e.g. WritableVirtualMachineWithConfigContext):

"platform": {
          "title": "Platform",
          "type": "integer",
          "x-nullable": true
        },

which translates to

	// Platform
	Platform *int64 `json:"platform,omitempty"`

which in turn will cause Go's json marshaller to omit empty values, which is intended behavior.

This is explained further in https://goswagger.io/faq/faq_model.html#non-required-or-nullable-property

A solution
One possible solution which I employ in a personal fork of go-netbox is to add

        "platform": {
          "title": "Platform",
          "type": "integer",
          "x-nullable": true,
          "x-omitempty": false
        },

which will remove the omitempty annotation from the Go struct.

The problem with this solution is that it requires post-procession of the netbox-generated swaggerfile for all optional attributes.
I do not know the stance of this repo's maintainers about post-processing the swagger.json file. It definitely feels less "pure" when compared to just fetching the swagger.json from netbox and regenerating the client. On the other hand, a client that cannot update certain attributes to their empty value is really not valuable.

Ideally, this would be fixed upstream by making netbox return a correct swagger file, but their swagger support is on a best-effort basis only.

@v0ctor
Copy link
Collaborator

v0ctor commented Feb 25, 2023

This issue is related to the way the code is implemented from the OpenAPI specification by go-swagger.

We have to migrate this project from go-swagger to openapi-generator for Netbox 3.5. Maybe this will fix the issue.

I leave the issue open until further notice.

@FlxPeters
Copy link

@fbreckle we should try this. Would be nice to get rid of the custom go netbox client

@BegBlev
Copy link

BegBlev commented Mar 23, 2023

@fbreckle, @FlxPeters it seems the problem comes from the use of omitempty with PATCH HTTP method. omitempty says "don't send the parameter if it is empty" the seconf says "Only uptade the parameters I send to you". Hence, empty parameters are not updated.

I saw in go-netbox code that each "update" fonction exists in 2 "flavors":

"full" update uses "PUT" method
"partial" update uses "PATCH" method

I don't really understand why those 2 falvors and when should we use one or the other. But I wonder if this wouldn't be the solution. When you have omitempty parameters you should probably use "full" update function. Isn't it?

Thank you
Vince

@zeddD1abl0
Copy link

I don't really understand why those 2 flavors and when should we use one or the other.

A "full" update SETS the object to the values you send in the request. Anything that isn't included is set to null. A "partial" update only updates the fields you send, and won't update anything that's outside the request.

If you send a request with (in JSON cause lazy):

{
"id":1,
"name":"Switch01",
"asset_tag":"123456"
}

A PUT request will set the name and asset_tag as per the request, and also set all the other values to null. It is PUTting the update over the top of the previous object.

A PATCH request will set the name and asset_tag as per the request, and not touch another value. It is PATCHing the original object.

Essentially, use PATCH when you want to adjust something, and PUT when you want to replace something.

@BegBlev
Copy link

BegBlev commented Apr 11, 2023

Hi @zeddD1abl0 and thank you for your reply,
I know the difference between a PUT and PUSH, my question was about when or what for using "full" or "partial" update flavors of the library?
From what I understand, if you know exactly which filed you will change, for instance using a form, and you know there are no "omitempty" fields then you should use the "partial" update flavor. If some of the field you can modify are "omitempty" fields then you must use "full" update flavor.

Hence, if my understanding is right, probably Netbox Terraform provider (which uses go-netbox library) should use "full" update.

@v0ctor
Copy link
Collaborator

v0ctor commented Jan 18, 2024

A new alpha version has been released with a different software to generate the library, so hopefully this bug has been resolved.

Please feel free to test it and to provide feedback.

@v0ctor v0ctor closed this as completed Jan 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants