From bce86701685080584103f30c6def455545951436 Mon Sep 17 00:00:00 2001 From: Justin Palmer Date: Thu, 12 Sep 2024 08:16:06 -0500 Subject: [PATCH 1/3] Optional Connection upon Client Instantiation This allows a user to create a client without _knowing_ the IP they are connecting to up front. It gives the user the ability to create and pass around a client object as needed. Additionally, the default behavior is preserved. * Added `check_connectivity` keyword arg to `RestClientBase`. * Added `check_connectivity` keyword arg to `HTTPClient`. * Added `check_connectivity` keyword arg to `redfish_client` function. * Conditionally execute `get_root_object` during client instantiation based on the value of `check_connectivity`. * Catch the resultant `AttributeError` in the case that `self.root` was not set due to `check_connectivity` being set to `False`. * Small test refactor, added `setUp` method to `TestRedFishClient` class to make commonly used attributes members of the test case. * Added new test for this functionality. Signed-off-by: Justin Palmer --- src/redfish/rest/v1.py | 19 +++++++++------- tests/rest/test_v1.py | 51 ++++++++++++++++++++++++------------------ 2 files changed, 40 insertions(+), 30 deletions(-) diff --git a/src/redfish/rest/v1.py b/src/redfish/rest/v1.py index db9d0ec..96d45f2 100644 --- a/src/redfish/rest/v1.py +++ b/src/redfish/rest/v1.py @@ -454,7 +454,7 @@ class RestClientBase(object): def __init__(self, base_url, username=None, password=None, default_prefix='/redfish/v1/', sessionkey=None, capath=None, cafile=None, timeout=None, - max_retry=None, proxies=None): + max_retry=None, proxies=None, check_connectivity=True): """Initialization of the base class RestClientBase :param base_url: The URL of the remote system @@ -497,7 +497,9 @@ def __init__(self, base_url, username=None, password=None, self.default_prefix = default_prefix self.capath = capath self.cafile = cafile - self.get_root_object() + + if check_connectivity: + self.get_root_object() def __enter__(self): self.login() @@ -999,7 +1001,7 @@ def login(self, username=None, password=None, auth=AuthMethod.SESSION): message_item = search_message(resp, "Base", "PasswordChangeRequired") if not message_item is None: raise RedfishPasswordChangeRequiredError("Password Change Required\n", message_item["MessageArgs"][0]) - + if not self.__session_key and resp.status not in [200, 201, 202, 204]: if resp.status == 401: # Invalid credentials supplied @@ -1042,7 +1044,7 @@ def __init__(self, base_url, username=None, password=None, default_prefix='/redfish/v1/', sessionkey=None, capath=None, cafile=None, timeout=None, - max_retry=None, proxies=None): + max_retry=None, proxies=None, check_connectivity=True): """Initialize HttpClient :param base_url: The url of the remote system @@ -1071,11 +1073,12 @@ def __init__(self, base_url, username=None, password=None, password=password, default_prefix=default_prefix, sessionkey=sessionkey, capath=capath, cafile=cafile, timeout=timeout, - max_retry=max_retry, proxies=proxies) + max_retry=max_retry, proxies=proxies, + check_connectivity=check_connectivity) try: self.login_url = self.root.Links.Sessions['@odata.id'] - except KeyError: + except (KeyError, AttributeError): # While the "Links/Sessions" property is required, we can fallback # on the URI hardened in 1.6.0 of the specification if not found LOGGER.debug('"Links/Sessions" not found in Service Root.') @@ -1128,7 +1131,7 @@ def redfish_client(base_url=None, username=None, password=None, default_prefix='/redfish/v1/', sessionkey=None, capath=None, cafile=None, timeout=None, - max_retry=None, proxies=None): + max_retry=None, proxies=None, check_connectivity=True): """Create and return appropriate REDFISH client instance.""" """ Instantiates appropriate Redfish object based on existing""" """ configuration. Use this to retrieve a pre-configured Redfish object @@ -1162,4 +1165,4 @@ def redfish_client(base_url=None, username=None, password=None, return HttpClient(base_url=base_url, username=username, password=password, default_prefix=default_prefix, sessionkey=sessionkey, capath=capath, cafile=cafile, timeout=timeout, - max_retry=max_retry, proxies=proxies) + max_retry=max_retry, proxies=proxies, check_connectivity=check_connectivity) diff --git a/tests/rest/test_v1.py b/tests/rest/test_v1.py index 4171893..c8da0cd 100644 --- a/tests/rest/test_v1.py +++ b/tests/rest/test_v1.py @@ -6,38 +6,45 @@ # -*- encoding: utf-8 -*- import unittest -from redfish.rest.v1 import HttpClient -from redfish.rest.v1 import RetriesExhaustedError -from redfish.rest.v1 import redfish_client +from redfish.rest.v1 import HttpClient, RetriesExhaustedError, redfish_client class TestRedFishClient(unittest.TestCase): + def setUp(self): + self.base_url = "http://foo.bar" + self.username = "rstallman" + self.password = "123456" + self.default_prefix = "/custom/redfish/v1/" + self.sessionkey = "fg687glgkf56vlgkf" + self.capath = "/path/to/the/dir" + self.cafile = "filename.test" + self.timeout = 666 + self.max_retry = 42 + def test_redfish_client(self): - base_url = "http://foo.bar" - username = "rstallman" - password = "123456" - default_prefix = "/custom/redfish/v1/" - sessionkey = "fg687glgkf56vlgkf" - capath = "/path/to/the/dir" - cafile = "filename.test" - timeout = 666 - max_retry = 42 # NOTE(hberaud) the client try to connect when we initialize the # http client object so we need to catch the retries exception first. # In a second time we need to mock the six.http_client to simulate # server responses and do some other tests with self.assertRaises(RetriesExhaustedError): - client = redfish_client(base_url=base_url) + client = redfish_client(base_url=self.base_url) # Check the object type self.assertTrue(isinstance(client, HttpClient)) # Check the object attributes values. # Here we check if the client object is properly initialized - self.assertEqual(client.base_url, base_url) - self.assertEqual(client.username, username) - self.assertEqual(client.password, password) - self.assertEqual(client.default_prefix, default_prefix) - self.assertEqual(client.sessionkey, sessionkey) - self.assertEqual(client.capath, capath) - self.assertEqual(client.cafile, cafile) - self.assertEqual(client.timeout, timeout) - self.assertEqual(client.max_retry, max_retry) + self.assertEqual(client.base_url, self.base_url) + self.assertEqual(client.username, self.username) + self.assertEqual(client.password, self.password) + self.assertEqual(client.default_prefix, self.default_prefix) + self.assertEqual(client.sessionkey, self.sessionkey) + self.assertEqual(client.capath, self.capath) + self.assertEqual(client.cafile, self.cafile) + self.assertEqual(client.timeout, self.timeout) + self.assertEqual(client.max_retry, self.max_retry) + + def test_redfish_client_no_root_resp(self): + client = redfish_client(base_url=self.base_url, check_connectivity=False) + self.assertIsNone(getattr(client, "root_resp", None)) + +if __name__ == '__main__': + unittest.main() From 29f0bab51272bbb6aa7485396519d9c37c0b689e Mon Sep 17 00:00:00 2001 From: Justin Palmer Date: Thu, 12 Sep 2024 09:35:24 -0500 Subject: [PATCH 2/3] Issue #163: Feedback Update * Ensure that the `root` data/response added as attributes by the `get_root_object` method are cached when `login` is called. * Added new test to cover this case. Signed-off-by: Justin Palmer --- src/redfish/rest/v1.py | 2 ++ tests/rest/test_v1.py | 37 +++++++++++++++++++++++++++++++++---- 2 files changed, 35 insertions(+), 4 deletions(-) diff --git a/src/redfish/rest/v1.py b/src/redfish/rest/v1.py index 96d45f2..0e0b60b 100644 --- a/src/redfish/rest/v1.py +++ b/src/redfish/rest/v1.py @@ -967,6 +967,8 @@ def login(self, username=None, password=None, auth=AuthMethod.SESSION): :type auth: object/instance of class AuthMethod """ + if getattr(self, "root_resp", None) is None: + self.get_root_object() self.__username = username if username else self.__username self.__password = password if password else self.__password diff --git a/tests/rest/test_v1.py b/tests/rest/test_v1.py index c8da0cd..c953c04 100644 --- a/tests/rest/test_v1.py +++ b/tests/rest/test_v1.py @@ -4,13 +4,15 @@ # https://github.com/DMTF/python-redfish-library/blob/main/LICENSE.md # -*- encoding: utf-8 -*- +import json import unittest +from unittest import mock from redfish.rest.v1 import HttpClient, RetriesExhaustedError, redfish_client class TestRedFishClient(unittest.TestCase): - def setUp(self): + def setUp(self) -> None: self.base_url = "http://foo.bar" self.username = "rstallman" self.password = "123456" @@ -21,7 +23,7 @@ def setUp(self): self.timeout = 666 self.max_retry = 42 - def test_redfish_client(self): + def test_redfish_client(self) -> None: # NOTE(hberaud) the client try to connect when we initialize the # http client object so we need to catch the retries exception first. # In a second time we need to mock the six.http_client to simulate @@ -42,9 +44,36 @@ def test_redfish_client(self): self.assertEqual(client.timeout, self.timeout) self.assertEqual(client.max_retry, self.max_retry) - def test_redfish_client_no_root_resp(self): + def test_redfish_client_no_root_resp(self) -> None: client = redfish_client(base_url=self.base_url, check_connectivity=False) self.assertIsNone(getattr(client, "root_resp", None)) -if __name__ == '__main__': + @mock.patch("requests.Session.request") + def test_redfish_client_root_object_initialized_after_login( + self, mocked_request: mock.Mock + ) -> None: + dummy_root_data = '{"Links": {"Sessions": {"@data.id": "/redfish/v1/SessionService/Sessions"}}}' + dummy_session_response = ( + '{"@odata.type": "#Session.v1_1_2.Session", ' + '"@odata.id": "/redfish/v1/SessionService/Sessions/1", ' + '"Id": "1", "Name": "User Session", "Description": "Manager User Session", ' + '"UserName": "user", "Oem": {}}' + ) + root_resp = mock.Mock(content=dummy_root_data, status_code=200) + auth_resp = mock.Mock( + content=dummy_session_response, + status_code=200, + headers={"location": "fake", "x-auth-token": "fake"}, + ) + mocked_request.side_effect = [ + root_resp, + auth_resp, + ] + client = redfish_client(base_url=self.base_url, check_connectivity=False) + client.login() + + self.assertEqual(client.root, json.loads(dummy_root_data)) + + +if __name__ == "__main__": unittest.main() From 72a23d7daffa4d5b6649edd0e14fe8103b916f3b Mon Sep 17 00:00:00 2001 From: Justin Palmer Date: Thu, 12 Sep 2024 14:00:00 -0500 Subject: [PATCH 3/3] Issue #163: Feedback Update * Updated docstrings to describe `check_connectivity` parameter. * Updated `README.rst` to call it out as an optional parameter. Signed-off-by: Justin Palmer --- README.rst | 1 + src/redfish/rest/v1.py | 9 +++++++++ 2 files changed, 10 insertions(+) diff --git a/README.rst b/README.rst index 65ccb77..2a30172 100644 --- a/README.rst +++ b/README.rst @@ -91,6 +91,7 @@ There are several optional parameters: * ``timeout``: The number of seconds to wait for a response before closing the connection. The default value is ``None``. * ``max_retry``: The number of retries to perform an operation before giving up. The default value is ``10``. * ``proxies``: A dictionary containing protocol to proxy URL mappings. The default value is ``None``. See `Using proxies`_. +* ``check_connectivity``: A boolean value to determine whether the client immediately attempts a connection to the base_url. The default is ``True``. To create a Redfish object, call the ``redfish_client`` method: diff --git a/src/redfish/rest/v1.py b/src/redfish/rest/v1.py index 0e0b60b..fd54842 100644 --- a/src/redfish/rest/v1.py +++ b/src/redfish/rest/v1.py @@ -477,6 +477,9 @@ def __init__(self, base_url, username=None, password=None, :type max_retry: int :param proxies: Dictionary containing protocol to proxy URL mappings :type proxies: dict + :param check_connectivity: A boolean to determine whether the client immediately checks for + connectivity to the base_url or not. + :type check_connectivity: bool """ @@ -1069,6 +1072,9 @@ def __init__(self, base_url, username=None, password=None, :type max_retry: int :param proxies: Dictionary containing protocol to proxy URL mappings :type proxies: dict + :param check_connectivity: A boolean to determine whether the client immediately checks for + connectivity to the base_url or not. + :type check_connectivity: bool """ super(HttpClient, self).__init__(base_url, username=username, @@ -1158,6 +1164,9 @@ def redfish_client(base_url=None, username=None, password=None, :type max_retry: int :param proxies: Dictionary containing protocol to proxy URL mappings :type proxies: dict + :param check_connectivity: A boolean to determine whether the client immediately checks for + connectivity to the base_url or not. + :type check_connectivity: bool :returns: a client object. """