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

Fix (#141) #144

Merged
merged 1 commit into from
Oct 24, 2023
Merged

Fix (#141) #144

merged 1 commit into from
Oct 24, 2023

Conversation

pszulczewski
Copy link
Contributor

@pszulczewski pszulczewski commented Oct 18, 2023

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

@pszulczewski
Copy link
Contributor Author

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

@pszulczewski pszulczewski mentioned this pull request Oct 18, 2023
@nautics889
Copy link
Contributor

nautics889 commented Oct 19, 2023

@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 (18f2717) 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_fieldsget_custom_fields and custom_field_choicesget_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
Copy link
Contributor Author

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

@pszulczewski pszulczewski merged commit 5987da4 into develop Oct 24, 2023
8 checks passed
@pszulczewski pszulczewski deleted the fix-141 branch October 24, 2023 13:25
pszulczewski added a commit that referenced this pull request Oct 24, 2023
* fix: Update methods for custom fields (#114)

Add method `custom_field_choices()` in `App` class.
Rename method `custom_choices()` to `custom_fields()`.
Update docstings for both methods according to returning data.

* fix: Tests for methods for custom fields (#114)

Add test case `AppCustomFieldChoicesTestCase` for
`custom_field_choices()` method.
Rename test case for `custom_fields()` method.
Add using fixtures for mentioned test cases.

* fix: Fixtures for tests (custom fields) (#114)

Add missing JSON files with fixtures for tests for getting custom
fields.

* fix: Failing test for getting custom fields (#114)

Fix test cases `AppCustomFieldsTestCase` and
`AppCustomFieldChoicesTestCase` for python 3.7 stable.
Update logic for checking passed arguments of mock's `call_args`.

* Update api.py

* max_workers added

* max_workers added

* Update query.py

* Update query.py

* max_workers from api to request

* debug removed

* Update api.py

* Update query.py

* refactor: Update docstrings and naming (#114)

* (docs) update docstrings for `custom_fields()` and
  `custom_field_choices()` methods
* (refactor): use f-strings instead of `.format()` in
  `custom_fields()` and `custom_field_choices()`
* (tests): update naming in tests
* (tests): use `return_value` instead of `side_effect` for mocks

* fix: backward compatibility

* (fix): restore original method `custom_choices()` for application
  class to provide a compatibility with existing client code
* (enhance): add logging a deprecation warning

* refactor: update `custom_choices()` (#114)

* (refactor): call `custom_fields()` in `custom_choices()` since they
  represent essentially identical requests to Nautobot

* Add version constraint to divide into two release trains 1.x and 2.x (#130)

Add version constraint to divide into two release trains 1.x and 2.x

* Release 2.0 (#136)

* Release 2.0.0

---------

Co-authored-by: Jan Snasel <jan.snasel@networktocode.com>
Co-authored-by: Joe Wesch <10467633+joewesch@users.noreply.github.com>

* Fix SSL verify (#140) (#142)

* Fix custom_fields and custom_field_choices method overlap with endpoints (#141) (#144)

* Release 2.0.1

---------

Co-authored-by: nautics889 <cyberukr@gmail.com>
Co-authored-by: NobodyIsPerfect78 <46317624+NobodyIsPerfect78@users.noreply.github.com>
Co-authored-by: Josh VanDeraa <jv@networktocode.com>
Co-authored-by: Joe Wesch <10467633+joewesch@users.noreply.github.com>
Co-authored-by: Jan Snasel <jan.snasel@networktocode.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

custom field endpoint
3 participants