search

nautobot / pynautobot

Nautobot Python SDK
https://pynautobot.readthedocs.io/en/latest/index.html
Apache License 2.0
36 stars 32 forks source link

Fix (#141) #144

Closed pszulczewski closed 10 months ago

pszulczewski commented 10 months ago

Add get to methods to fix custom_fields and custom_field_choices methods overlap with endpoints. Fixes #141

pszulczewski commented 10 months ago

FYI we had to add get to the methods you added as they overlap with endpoint names, if you're using them please append get. //cc @nautics889

nautics889 commented 10 months ago

@pszulczewski

Apologies for this oversight. Seems like that case has been undertested from my side. Indeed, if we run something like

import pynautobot
nb = pynautobot.api(
    url="https://demo.nautobot.com/",
    token="aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",
)

print(repr(nb.extras.custom_fields))
print(repr(nb.extras.__getattr__("custom_fields")))

it produces the next output:

<bound method App.custom_choices of <pynautobot.core.app.App object at 0x7f233ca46680>>  # this one is what a client actually get, an `App` instance
<pynautobot.core.endpoint.Endpoint object at 0x7f233ca46da0>  # this one is what a client expects to get, an `Enpoint`

Meanwhile, with the old implementation of .custom_fields(), when it was named .custom_choices() in v1.5.0 (18f2717e33f4518be2d8bacc0f3068582ca5ecb0) the Endpoint instance was available to be gotten by nb.extras.custom_fields (because App class didn't have this method and __getattr__ was called then, returning Endpoint instance). And the script above would print pynautobot.core.endpoint.Endpoint in both cases, like it should print. This point makes me think that these difference in naming (custom_choices for fetching custom_fields exactly) has been made intentionally then. However, I'm still thinking that the naming approach wasn't fully correct then (in older versions), since (like i claimed before) this can cause /api/extras/custom-fields/ and /api/extras/custom-field-choices/ to be confused. But i must have discovered that inability of getting custom_fields endpoint.

I think the renaming (custom_fields→get_custom_fields and custom_field_choices→get_custom_field_choices) is a great solution for those issues: this lets the user to get according Endpoint instances and keeps /api/extras/custom-fields/ & /api/extras/custom-field-choices/ different. Thank you a lot for fixing.

pszulczewski commented 10 months ago

No issues ;-) it's great that you contributed to the library.