search

netbox-community / pynetbox

Python API client library for Netbox.
Apache License 2.0
561 stars 168 forks source link

Updating object with JSON custom field causes a traceback #457

Closed jcollie closed 1 year ago

jcollie commented 2 years ago

I have a JSON custom field on some objects in my NetBox 3.2beta instance. When trying to update one of the objects I get this traceback.

Traceback (most recent call last):
  File "/home/jcollie/dev/netbox-maintenance/fix-model-aliases.py", line 602, in <module>
    main()
  File "/usr/lib/python3.10/site-packages/click/core.py", line 1137, in __call__
    return self.main(*args, **kwargs)
  File "/usr/lib/python3.10/site-packages/click/core.py", line 1062, in main
    rv = self.invoke(ctx)
  File "/usr/lib/python3.10/site-packages/click/core.py", line 1404, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File "/usr/lib/python3.10/site-packages/click/core.py", line 763, in invoke
    return __callback(*args, **kwargs)
  File "/home/jcollie/dev/netbox-maintenance/fix-model-aliases.py", line 597, in main
    if device_type.save():
  File "/home/jcollie/.local/lib/python3.10/site-packages/pynetbox/core/response.py", line 529, in save
    updates = self.updates()
  File "/home/jcollie/.local/lib/python3.10/site-packages/pynetbox/core/response.py", line 506, in updates
    diff = self._diff()
  File "/home/jcollie/.local/lib/python3.10/site-packages/pynetbox/core/response.py", line 484, in _diff
    {fmt_dict(k, v) for k, v in self.serialize(init=True).items()}
  File "/home/jcollie/.local/lib/python3.10/site-packages/pynetbox/core/response.py", line 457, in serialize
    ret[i] = flatten_custom(current_val)
  File "/home/jcollie/.local/lib/python3.10/site-packages/pynetbox/core/response.py", line 61, in flatten_custom
    return {
  File "/home/jcollie/.local/lib/python3.10/site-packages/pynetbox/core/response.py", line 62, in <dictcomp>
    k: v if not isinstance(v, dict) else v["value"] for k, v in custom_dict.items()
KeyError: 'value'
markkuleinio commented 2 years ago

Testing code:

>>> device = netbox.dcim.devices.get(1)
>>> pprint.pprint(device.custom_fields)
{'json': {'here': 123, 'some': 'values'}}
>>> device.custom_fields["json"] = {"update": "other", "values": True}
>>> device.save()
Traceback (most recent call last):
...
    k: v if not isinstance(v, dict) else v["value"] for k, v in custom_dict.items()
KeyError: 'value'

With a text custom field it works.

TBH, I don't get the purpose of "k: v if not isinstance(v, dict) else v["value"]" in flatten_custom(): basically it says that if the custom field data is a dict, then it only returns the value item of it, not the full dict. And I don't see a case when this value key is relevant. Any idea?

Removing the call to flatten_custom() altogether (= using just ret[i] = current_val in Record.serialize()) gets JSON data saving to work. But I don't know when this value thingy would be needed.

https://github.com/netbox-community/pynetbox/blob/0f1e588a367e6b7f375ce0c863c1cf784ac462cc/pynetbox/core/response.py#L453-L458

jcollie commented 2 years ago

TBH, I don't get the purpose of "k: v if not isinstance(v, dict) else v["value"]" in flatten_custom(): basically it says that if the custom field data is a dict, then it only returns the value item of it, not the full dict. And I don't see a case when this value key is relevant. Any idea?

I think it is for the case of objects that have a "label" and a "value" like the Status entry in many objects like Device.

markkuleinio commented 2 years ago

I think it is for the case of objects that have a "label" and a "value" like the Status entry in many objects like Device.

But flatten_custom() is only used for custom fields, see the line #456 in the snippet above 🤔

jcollie commented 2 years ago

But flatten_custom() is only used for custom fields, see the line #456 in the snippet above thinking

Not saying it makes sense, just trying to guess what the author may have been thinking when they wrote that particular piece of code.

avolmensky commented 2 years ago

I'm also seeing the same bug. I have also removed the call to flatten_custom() and it seems to be working.

Edit: After doing a bunch of digging, I really can't see why this value key is used?

mrlocke commented 2 years ago

Thanks @rodvand. I've tested the solution proposed above by markkuleinio and now everything works. Not sure if it may break something else, but up to the moment all other automations I've works fine.

It will be really nice if this can be included in pynetbox production release.

srfwx commented 2 years ago

Hi, I've started using custom fields of type objects on my NetBox instance. The flatten_custom() is definitely needed to allow pynetbox to update such records, but I had to modify it like this: k: v if not isinstance(v, dict) else v["id"] for k, v in custom_dict.items() I am not sure either in which case we could encounter a Dict with a "value" key in a custom field, but the problem with the solution above is that it probably breaks things again for json fields. I didn't look further yet but I think we would need to look at the custom field type in order to flatten it properly? Or not at all if it's a JSON. Edit: We probably can't have this information from the API. So a more elaborate flatten_custom() would be needed with key id and value not allowed in JSON custom fields? k: v if not isinstance(v, dict) or not v.get("id") else v["id"] for k, v in custom_dict.items() Edit2: We can get the field type from /extras/custom-fields/ but that requires extra API calls. Options were already discussed here https://github.com/netbox-community/pynetbox/issues/436#issuecomment-1025140828

falkowich commented 1 year ago

Any workaround for this that you know of? I am stuck at updating our objects now when we use custom_fields as objects.

-- Kind Regards Falk

Kani999 commented 1 year ago

Any workaround for this that you know of? I am stuck at updating our objects now when we use custom_fields as objects.

-- Kind Regards Falk

I've forked the repository and did a small change https://github.com/Kani999/pynetbox/tree/fix_cf_6.6.2

Then I released the forked version to pypi, so I can simply install it in my projects.

I'm trying to maintain the version with the original pynetbox, but I don't guarantee anything in the future

falkowich commented 1 year ago

I'm trying to maintain the version with the original pynetbox, but I don't guarantee anything in the future

Thanks, going to use is for the moment, until you want to deprecate or stale it!

-- Kind Regards Falk

FliesLikeABrick commented 1 year ago

@Kani999 can you open a new PR for your fix to come back upstream to pynetbox, so that someone can hopefully review that proposed fix? I know the last one stalled, but hopefully we can keep trying to nudge the right people to help gain consensus on the proper fix

Kani999 commented 1 year ago

Pull Request #518

nomad-cyanide commented 1 year ago

I have run into to this error as well: We are populating Netbox with a number of custom fields and we wanted to have another custom field to our virtual machines, show which other virtual machine is managing it. Manually in gui, it works fine, but when our existing scripts try to update a VM that has something in the managed_by field, it fails with the:

File "/usr/local/lib/python3.6/site-packages/pynetbox/core/response.py", line 62, in k: v if not isinstance(v, dict) else v["value"] for k, v in custom_dict.items() KeyError: 'value'

If the managed_by field is empty, no errors occur.

The field is defined like this:

{
    "id": 16,
    "url": "https://netbox.test/api/extras/custom-fields/16/",
    "display": "Managed by",
    "content_types": [
        "virtualization.virtualmachine"
    ],
    "type": {
        "value": "object",
        "label": "Object"
    },
    "object_type": "virtualization.virtualmachine",
    "data_type": "object",
    "name": "managed_by",
    "label": "Managed by",
    "group_name": "Checkpoint",
    "description": "Indicates which other virtual machine is managing this VM",
    "required": false,
    "filter_logic": {
        "value": "loose",
        "label": "Loose"
    },
    "ui_visibility": {
        "value": "read-write",
        "label": "Read/Write"
    },
    "default": null,
    "weight": 100,
    "validation_minimum": null,
    "validation_maximum": null,
    "validation_regex": "",
    "choices": [],
    "created": "2023-01-30T14:28:32.026308+01:00",
    "last_updated": "2023-01-30T14:28:32.026325+01:00"
}

Pynetbox is version version 7.0.0 Netbox is version 3.3.2

srfwx commented 1 year ago

Try upgrading Pynetbox to v7.0.1 This should have been fixed

abhi1693 commented 1 year ago

Fixed by #518