netbox-community / pynetbox

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

ansible netbox custom_fields response error #497

Closed pat-xd closed 1 year ago

pat-xd commented 2 years ago

Hi,

I'm trying to manage netbox with ansible and get an error, when my device has a custom_field with JSON. This is my custom field and value

 custom_fields:
              attributes: {"hw":{"memory_mb":"{{ memory_mb }}", "cpu": "{{ cpu }}", "disk_gb": "{{ disk_gb }}"} }

pynetbox/pynetbox/core/response.py:62 k: v if not isinstance(v, dict) else v[\"value\"] for k, v in custom_dict.items() KeyError: 'value'

I dont unterstand the else path v["value"] When does the dict have a key "value"?

My fix was to remove the v["value"]

index 190c292..f63c2b1 100644
--- a/pynetbox/core/response.py
+++ b/pynetbox/core/response.py
@@ -59,7 +59,7 @@ def get_return(lookup, return_fields=None):

 def flatten_custom(custom_dict):
     return {
-        k: v if not isinstance(v, dict) else v["value"] for k, v in custom_dict.items()
+        k: v for k, v in custom_dict.items()
     }
endreszabo commented 2 years ago

I encountered the same today 3 am and was about to report. :) Thanks.

endreszabo commented 2 years ago

It happened to me when I wanted to edit a custom field of a prefix. Turns out it fails even with an empty dict as well:

>>> p.save
<bound method Record.save of fd4d:4045:e5e8:400::/56>
>>> p.save()
Traceback (most recent call last):
  File "<input>", line 1, in <module>
    p.save()
  File "/usr/lib/python3.10/site-packages/pynetbox/core/response.py", line 529, in save
    updates = self.updates()
  File "/usr/lib/python3.10/site-packages/pynetbox/core/response.py", line 506, in updates
    diff = self._diff()
  File "/usr/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 "/usr/lib/python3.10/site-packages/pynetbox/core/response.py", line 457, in serialize
    ret[i] = flatten_custom(current_val)
  File "/usr/lib/python3.10/site-packages/pynetbox/core/response.py", line 61, in flatten_custom
    return {
  File "/usr/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'
endreszabo commented 2 years ago

@pat-xd I'm not sure under what circumstances a custom field dict's value is a dict having a value field, but to stay on the safe side, I'd rewrite the line in question as:

-        k: v if not isinstance(v, dict) else v["value"] for k, v in custom_dict.items()
+        k: v if not isinstance(v, dict) else v.get("value", v) for k, v in custom_dict.items()¶

But of course, the unittest will tell. At least it works for my use cases and feels backward compatible with the value being a dict having a value key.

Kani999 commented 2 years ago

Related - #472

paulexyz commented 1 year ago

@endreszabo This happens for all custom fields with the data_type is set to object, here for example I added a reference to a virtual machine

# section of GET response for object

"custom_fields": {
  # fields of type string
  "custom_string": "foobar",
  # field of type opject
  "linked_vm": {
    "id": 42,
    "url": "http://192.0.2.0/api/virtualization/virtual-machines/42/",
    "display": "vmmaster",
    "name": "vmmaster"
  }
}
paulexyz commented 1 year ago

I also run the test suite with your proposed change @endreszabo, no errors there - for me that change also looks like the right thing to do

paulexyz commented 1 year ago

FYI @pat-xd and @endreszabo - I built a for for meanwhile usuage on pypi until this issue is resolved - I needed that fixed immediately

paulexyz commented 1 year ago

Can be closed because it should be fixed by #518 and is released in 7.0.1