Update pynetbox to new tags in Netbox 2.9

[fabi125]

Since Netbox 2.9 tags are returned as nested records instead of just flat strings.

This PR updates pynetbox to correctly handle this new situation.

AFICT this should be backwards compatible.

[zachmoody]

Seems we would at least break what's preventing duplicate tags from being sent for pre-2.9 NetBox clients. I haven't had a chance to see what the behavior is like there with the current version either. Do you happen to know? Also we'd probably not want to alter the existing tests so we can ensure backwards compatibility and just make new ones for the 2.9+ behavior.

[fabi125]

This is the current behaviour when connecting to Netbox 2.9.3:

In [10]: device.tags.append("foo")

In [11]: device.save()
---------------------------------------------------------------------------
RequestError                              Traceback (most recent call last)
<ipython-input-11-69b3c1babe60> in <module>
----> 1 device.save()

/home/fabi/testvenv/lib/python3.6/site-packages/pynetbox/core/response.py in save(self)
    373                     ssl_verify=self.api.ssl_verify,
    374                 )
--> 375                 if req.patch({i: serialized[i] for i in diff}):
    376                     return True
    377

/home/fabi/testvenv/lib/python3.6/site-packages/pynetbox/core/query.py in patch(self, data)
    343             return req.json()
    344         else:
--> 345             raise RequestError(req)

RequestError: The request failed with code 400 Bad Request: {'tags': [{}, ['Related objects must be referenced by numeric ID or by dictionary of attributes. Received an unrecognized value: foo']]}

And then when you try to use a dict:

In [16]: device.tags.append({"name": "foo"})

In [17]: device.save()
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-17-69b3c1babe60> in <module>
----> 1 device.save()

/home/fabi/testvenv/lib/python3.6/site-packages/pynetbox/core/response.py in save(self)
    379         """
    380         if self.id:
--> 381             diff = self._diff()
    382             if diff:
    383                 serialized = self.serialize()

/home/fabi/testvenv/lib/python3.6/site-packages/pynetbox/core/response.py in _diff(self)
    355             return k, v
    356
--> 357         current = Hashabledict({fmt_dict(k, v) for k, v in self.serialize().items()})
    358         init = Hashabledict(
    359             {fmt_dict(k, v) for k, v in self.serialize(init=True).items()}

/home/fabi/testvenv/lib/python3.6/site-packages/pynetbox/core/response.py in serialize(self, nested, init)
    343                     ]
    344                     if i in LIST_AS_SET:
--> 345                         current_val = list(OrderedDict.fromkeys(current_val))
    346                 ret[i] = current_val
    347         return ret

TypeError: unhashable type: 'dict'

With this fix and Netbox 2.8.9, if you specify duplicate tags, the server will just dedupe them for you. Nothing bad seems to happen as far as i can tell.

A hacky solution would be to de-dupe things that are hashable, and leave everything else as is. If this is something you'd like, I can update the PR.

[zachmoody]

With this fix and Netbox 2.8.9, if you specify duplicate tags, the server will just dedupe them for you. Nothing bad seems to happen as far as i can tell.

Nice!

A hacky solution would be to de-dupe things that are hashable, and leave everything else as is. If this is something you'd like, I can update the PR.

hrm, maybe, I suspect the problem stems, in some part, from the default behavior here: https://github.com/digitalocean/pynetbox/blob/c45411a8b5749537baa91fb6bcc1321fa144a233/pynetbox/core/response.py#L342
Since (I don't think) these are record objects, we're just copying the list of dicts and causing the TypeError when we call fromkeys(). Best way to deal with that isn't immediately clear, if that's even what's happening.

[fabi125]

Since (I don't think) these are record objects, we're just copying the list of dicts and causing the TypeError when we call fromkeys(). Best way to deal with that isn't immediately clear, if that's even what's happening.

That is correct. They were either strings or dicts in my tests. If you fetch a device from 2.9+ that contains existing tags, those will be Records, so a save() on those works. It also works if you put in the Record ID directly or fetch the Record from the API separately and add it to the list.

How would you like to proceed here?

[smutel]

Any news on this MR ?

[daemonkeeper]

Right, please give an update if you intend to merge this anytime soon. I too am in a situation to port my applications to NB 2.9, and for that I need to know how the tag support is going to look like. Thanks.

[zachmoody]

No, it needs some tweaks before merging. We'd, at least, need to continue testing the pre-2.9 behavior with strings. I'm not so sure we want to offload tag deduplication to the server either (assuming the 2.8+ behavior applies for earlier versions as well). In cases where client code was simply appending a tag we'd send an update every time it was run.

[daemonkeeper]

I'm not sure this needs to be adressed. As a developer, I would consider it my responsibility to ensure which tags I wish to associate with an interface. This is, I would consider it my job to ensure I don't send duplicated tags to pynetbox/netbox if I don't want to see dupes. Besides, this anyway seems to be already the case as a convenient feature of Netbox itself.

Moreover, I'd like to emphasize that right now pynetbox is not usable at all with NB 2.9, but the opposite is not true alike. If you'd be merging this code and this would cause duplication problems with outdated versions of Netbox, people can still downgrade pynetbox until they fixed their code. The reverse is not true, leaving me with a broken app I cannot fix unless I want to monkey-patch your library.

[smutel]

Please push an update to manage Netbox 2.9.

[jbe-dw]

@zachmoody

Hi There,

It has been 3 months since this issue is present. Netbox community 2.10 beta is out and we're still stuck with 2.8. Could you please make a choice to let things move on ? If you don't have free time to work on the project, at least take a decision or let people know what should be done further to make it merged.

People not updating their Netbox instance probably don't update their cli SDKs as well. Specifying it could be an issue for deployment outdated can be good enough ?

[jbe-dw]

@zachmoody

                if isinstance(current_val, list):
                    if float(self.api.version) >= 2.9 and i == "tags":
                        current_val = [
                            v.id if isinstance(v, Record)
                            else v if isinstance(v, dict)
                            else { "name": v}
                            for v in current_val
                        ]
                    else:
                        current_val = [
                            v.id if isinstance(v, Record) else v for v in current_val
                        ]
                        if i in LIST_AS_SET:
                            current_val = list(OrderedDict.fromkeys(current_val))
                ret[i] = current_val
        return ret

Is a change like this acceptable ?

• We change the behavior for newer versions only about tags only
• The tags can be provided as a string (like before) or a dict (as it should now)
• Netbox >= 2.9 API only accept existing tags where previous versions accept any string and would create the tag. This means that even if the call is well formated it fails if the tag is not found.
• As @fabi125 said, Netbox does the dedup itself, it can be hardcoded in the SDK if you really think it's better.

I would take extra time to review the _diff() method as it sees a change when there's not for now.

Please take some minute to give an insight to this change so I can follow on with the diff (and deduplication if still wished). I'll therefore do a PR.

Greetings.

[markkuleinio]

@zachmoody Any comments on this? I (for one) have quite a huge application stack built on pynetbox, planning for NetBox 2.9+ upgrades. Are you still developing and commenting pynetbox to follow with NetBox development?

[zachmoody]

It has been 3 months since this issue is present. Netbox community 2.10 beta is out and we're still stuck with 2.8. Could you please make a choice to let things move on ? If you don't have free time to work on the project, at least take a decision or let people know what should be done further to make it merged.

I'll be the first to admit I haven't been able to put in the time the project's required lately. I do my best to respond to PRs for bugs and breaking changes like this one quickly, but I can't drop everything and fix them when they need work. I mentioned in my first comment that I can't merge this if we discontinue testing the pre-2.9 behavior. The implication there is that we'd need something to handle differentiating them. Your PR (#307) is one way, and I made a comment there mentioning what I'd like to do and when. I probably could've been more clear, sooner.

People not updating their Netbox instance probably don't update their cli SDKs as well.

That's probably not a bet I'd take 😄. I don't really want to get into the game of mapping pynetbox versions to supported versions of NetBox.

[zachmoody]

Thanks for the PR, this should be fixed in #311.