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

lazy-load api version upon first call rather than instantiation #225

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 7 additions & 6 deletions pynautobot/core/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
# limitations under the License.
#
# This file has been modified by NetworktoCode, LLC.

from functools import cached_property
from packaging import version
import requests
from requests.adapters import HTTPAdapter
Expand Down Expand Up @@ -110,15 +110,13 @@ def __init__(
self.users = App(self, "users")
self.plugins = PluginsApp(self)
self.graphql = GraphQLQuery(self)
self._validate_version()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By removing this in the init, we no longer validate the version unless the end user explicitly calls the property themselves now (barring a single method that I could find). Can we make every call get the .version property so it will trigger the _validate_version method on the first one and just return the cached property on every other one?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@brandomando on this one...


def _validate_version(self):
def _validate_version(self, api_version: str) -> None:
"""Validate API version if eq or ge than 2.0 raise an error."""
api_version = self.version
if api_version.replace(".", "").isnumeric() and version.parse(api_version) < version.parse("2.0"):
raise ValueError("Nautobot version 1 detected, please downgrade pynautobot to version 1.x")

@property
@cached_property
def version(self):
"""Retrieves the version of the Nautobot REST API that the connected instance is using.

Expand All @@ -142,12 +140,15 @@ def version(self):
'1.0'
"""

return Request(
v = Request(
base=self.base_url,
http_session=self.http_session,
api_version=self.api_version,
token=self.token,
).get_version()
self._validate_version(v)

return v

def openapi(self):
"""Retrieves the OpenAPI specification (OAS) document for the connected Nautobot instance.
Expand Down
33 changes: 28 additions & 5 deletions tests/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ class ResponseHeadersWithVersion:
)
def test_api_version(self, *_):
with self.assertRaises(ValueError) as error:
pynautobot.api(host)
pynautobot.api(host).version
self.assertEqual(
str(error.exception), "Nautobot version 1 detected, please downgrade pynautobot to version 1.x"
)
Expand Down Expand Up @@ -124,6 +124,31 @@ def test_api_status(self, *_):
self.assertEqual(api.status()["nautobot-version"], "1.3.2")


class ApiLazyLoadVersionTestCase(unittest.TestCase):
"""
Tests that version is lazy loaded on the first API call, rather than on instantiation
"""

@patch("urllib3.connectionpool.HTTPConnectionPool._get_conn")
def test_lazy_load_version(self, getconn_mock):
getconn_mock.return_value.getresponse.side_effect = [
Mock(status=200, msg=HTTPMessage()),
]

api = pynautobot.api(
"http://any.url/",
retries=2,
)

assert getconn_mock.return_value.request.mock_calls == []

api.version

assert getconn_mock.return_value.request.mock_calls == [
call("GET", "/api/", body=None, headers=ANY),
]


class ApiRetryTestCase(unittest.TestCase):
class ResponseWithStatus:
ok = False
Expand All @@ -137,12 +162,10 @@ def test_api_retry(self, getconn_mock):
Mock(status=200, msg=HTTPMessage()),
]

api = pynautobot.api(
pynautobot.api(
"http://any.url/",
retries=2,
)
with patch("pynautobot.api.version", "2.0"):
api.version
).version

assert getconn_mock.return_value.request.mock_calls == [
call("GET", "/api/", body=None, headers=ANY),
Expand Down