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

Updating object with JSON custom field causes a traceback #457

Closed
jcollie opened this issue Mar 31, 2022 · 15 comments
Closed

Updating object with JSON custom field causes a traceback #457

jcollie opened this issue Mar 31, 2022 · 15 comments
Labels
type: bug A confirmed report of unexpected behavior in the application

Comments

@jcollie
Copy link

jcollie commented Mar 31, 2022

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'
@zachmoody zachmoody added the type: bug A confirmed report of unexpected behavior in the application label Apr 16, 2022
@markkuleinio
Copy link
Contributor

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.

ret = {}
for i in dict(self):
current_val = getattr(self, i) if not init else init_vals.get(i)
if i == "custom_fields":
ret[i] = flatten_custom(current_val)
else:

@jcollie
Copy link
Author

jcollie commented Apr 18, 2022

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
Copy link
Contributor

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
Copy link
Author

jcollie commented Apr 18, 2022

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
Copy link

avolmensky commented May 10, 2022

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
Copy link

mrlocke commented Jun 13, 2022

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
Copy link
Contributor

srfwx commented Jun 21, 2022

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 #436 (comment)

@falkowich
Copy link

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
Copy link
Contributor

Kani999 commented Nov 16, 2022

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
Copy link

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
Copy link

FliesLikeABrick commented Nov 16, 2022

@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
Copy link
Contributor

Kani999 commented Nov 21, 2022

Pull Request #518

@nomad-cyanide
Copy link

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
Copy link
Contributor

srfwx commented Jan 31, 2023

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

@abhi1693
Copy link
Member

Fixed by #518

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug A confirmed report of unexpected behavior in the application
Projects
None yet
Development

Successfully merging a pull request may close this issue.