diff --git a/releasenotes/notes/fix-refine-resource-refresh-86c21ce230967251.yaml b/releasenotes/notes/fix-refine-resource-refresh-86c21ce230967251.yaml new file mode 100644 index 00000000..9005ae55 --- /dev/null +++ b/releasenotes/notes/fix-refine-resource-refresh-86c21ce230967251.yaml @@ -0,0 +1,6 @@ +--- +fixes: + - | + The library now supports reloading of the attributes by invoking + ``refresh()`` method for nested resources in contrast to recreation. + Resources can now be marked stale by invoking ``invalidate()``. diff --git a/sushy/resources/base.py b/sushy/resources/base.py index a6083ba4..5221b6c5 100644 --- a/sushy/resources/base.py +++ b/sushy/resources/base.py @@ -197,6 +197,11 @@ class ResourceBase(object): self._path = path self._json = None self.redfish_version = redfish_version + # Note(deray): Indicates if the resource holds stale data or not. + # Starting off with True and eventually gets set to False when + # attribute values are fetched. + self._is_stale = True + self.refresh() def _parse_attributes(self): @@ -205,20 +210,66 @@ class ResourceBase(object): # Hide the Field object behind the real value setattr(self, attr, field._load(self.json, self)) - def refresh(self): + def refresh(self, force=False): """Refresh the resource Freshly retrieves/fetches the resource attributes and invokes ``_parse_attributes()`` method on successful retrieval. + Advised not to override this method in concrete ResourceBase classes. + Resource classes can place their refresh specific operations in + ``_do_refresh()`` method, if needed. This method represents the + template method in the paradigm of Template design pattern. + + :param force: will force refresh the resource and its sub-resources, + if set to True. :raises: ResourceNotFoundError :raises: ConnectionError :raises: HTTPError """ + # Note(deray): Don't re-fetch / invalidate the sub-resources if the + # resource is "_not_ stale" (i.e. fresh) OR _not_ forced. + if not self._is_stale and not force: + return + self._json = self._conn.get(path=self._path).json() LOG.debug('Received representation of %(type)s %(path)s: %(json)s', {'type': self.__class__.__name__, 'path': self._path, 'json': self._json}) self._parse_attributes() + self._do_refresh(force) + + # Mark it fresh + self._is_stale = False + + def _do_refresh(self, force): + """Primitive method to be overridden by refresh related activities. + + Derived classes are supposed to override this method with the + resource specific refresh operations to be performed. This is a + primitive method in the paradigm of Template design pattern. + + :param force: should force refresh the resource and its sub-resources, + if set to True. + :raises: ResourceNotFoundError + :raises: ConnectionError + :raises: HTTPError + """ + + def invalidate(self, force_refresh=False): + """Mark the resource as stale, prompting refresh() before getting used. + + If ``force_refresh`` is set to True, then it invokes ``refresh()`` + on the resource. + + :param force_refresh: will invoke refresh on the resource, + if set to True. + :raises: ResourceNotFoundError + :raises: ConnectionError + :raises: HTTPError + """ + self._is_stale = True + if force_refresh: + self.refresh() @property def json(self): diff --git a/sushy/resources/system/ethernet_interface.py b/sushy/resources/system/ethernet_interface.py index 86baea7a..9911e1ff 100644 --- a/sushy/resources/system/ethernet_interface.py +++ b/sushy/resources/system/ethernet_interface.py @@ -79,6 +79,11 @@ class EthernetInterfaceCollection(base.ResourceCollectionBase): self._summary = mac_dict return self._summary - def refresh(self): - super(EthernetInterfaceCollection, self).refresh() + def _do_refresh(self, force=False): + """Do custom resource specific refresh activities + + On refresh, all sub-resources are marked as stale, i.e. + greedy-refresh not done for them unless forced by ``force`` + argument. + """ self._summary = None diff --git a/sushy/resources/system/processor.py b/sushy/resources/system/processor.py index 72fa416c..a4415c9a 100644 --- a/sushy/resources/system/processor.py +++ b/sushy/resources/system/processor.py @@ -120,8 +120,12 @@ class ProcessorCollection(base.ResourceCollectionBase): super(ProcessorCollection, self).__init__(connector, path, redfish_version) - def refresh(self): - """Refresh the resource""" - super(ProcessorCollection, self).refresh() + def _do_refresh(self, force=False): + """Do custom resource specific refresh activities + + On refresh, all sub-resources are marked as stale, i.e. + greedy-refresh not done for them unless forced by ``force`` + argument. + """ # Reset summary attribute self._summary = None diff --git a/sushy/resources/system/system.py b/sushy/resources/system/system.py index 7f081aeb..35d1ae42 100644 --- a/sushy/resources/system/system.py +++ b/sushy/resources/system/system.py @@ -248,25 +248,28 @@ class System(base.ResourceBase): @property def processors(self): - """Property to provide reference to `ProcessorCollection` instance + """Property to reference `ProcessorCollection` instance - It is calculated once when the first time it is queried. On refresh, - this property gets reset. + It is set once when the first time it is queried. On refresh, + this property is marked as stale (greedy-refresh not done). + Here the actual refresh of the sub-resource happens, if stale. """ if self._processors is None: self._processors = processor.ProcessorCollection( self._conn, self._get_processor_collection_path(), redfish_version=self.redfish_version) + self._processors.refresh(force=False) return self._processors - def refresh(self): - super(System, self).refresh() - self._processors = None - self._ethernet_interfaces = None - @property def ethernet_interfaces(self): + """Property to reference `EthernetInterfaceCollection` instance + + It is set once when the first time it is queried. On refresh, + this property is marked as stale (greedy-refresh not done). + Here the actual refresh of the sub-resource happens, if stale. + """ if self._ethernet_interfaces is None: self._ethernet_interfaces = ( ethernet_interface.EthernetInterfaceCollection( @@ -274,8 +277,21 @@ class System(base.ResourceBase): utils.get_sub_resource_path_by(self, "EthernetInterfaces"), redfish_version=self.redfish_version)) + self._ethernet_interfaces.refresh(force=False) return self._ethernet_interfaces + def _do_refresh(self, force=False): + """Do custom resource specific refresh activities + + On refresh, all sub-resources are marked as stale, i.e. + greedy-refresh not done for them unless forced by ``force`` + argument. + """ + if self._processors is not None: + self._processors.invalidate(force) + if self._ethernet_interfaces is not None: + self._ethernet_interfaces.invalidate(force) + class SystemCollection(base.ResourceCollectionBase): diff --git a/sushy/tests/unit/resources/system/test_processor.py b/sushy/tests/unit/resources/system/test_processor.py index 849c6e3c..0dd2dadc 100644 --- a/sushy/tests/unit/resources/system/test_processor.py +++ b/sushy/tests/unit/resources/system/test_processor.py @@ -140,7 +140,7 @@ class ProcessorCollectionTestCase(base.TestCase): with open('sushy/tests/unit/json_samples/' 'processor_collection.json', 'r') as f: self.conn.get.return_value.json.return_value = json.loads(f.read()) - self.sys_processor_col.refresh() + self.sys_processor_col.refresh(force=True) # | WHEN & THEN | self.assertIsNone(self.sys_processor_col._summary) diff --git a/sushy/tests/unit/resources/system/test_system.py b/sushy/tests/unit/resources/system/test_system.py index 6b237119..22eac5e3 100644 --- a/sushy/tests/unit/resources/system/test_system.py +++ b/sushy/tests/unit/resources/system/test_system.py @@ -290,10 +290,13 @@ class SystemTestCase(base.TestCase): # On refreshing the system instance... with open('sushy/tests/unit/json_samples/system.json', 'r') as f: self.conn.get.return_value.json.return_value = json.loads(f.read()) + + self.sys_inst.invalidate() self.sys_inst.refresh() # | WHEN & THEN | - self.assertIsNone(self.sys_inst._processors) + self.assertIsNotNone(self.sys_inst._processors) + self.assertTrue(self.sys_inst._processors._is_stale) # | GIVEN | with open('sushy/tests/unit/json_samples/processor_collection.json', @@ -302,6 +305,7 @@ class SystemTestCase(base.TestCase): # | WHEN & THEN | self.assertIsInstance(self.sys_inst.processors, processor.ProcessorCollection) + self.assertFalse(self.sys_inst._processors._is_stale) def _setUp_processor_summary(self): self.conn.get.return_value.json.reset_mock() diff --git a/sushy/tests/unit/resources/test_base.py b/sushy/tests/unit/resources/test_base.py index ff4e9dfa..678a4f4e 100644 --- a/sushy/tests/unit/resources/test_base.py +++ b/sushy/tests/unit/resources/test_base.py @@ -36,11 +36,27 @@ class ResourceBaseTestCase(base.TestCase): self.conn = mock.Mock() self.base_resource = BaseResource(connector=self.conn, path='/Foo', redfish_version='1.0.2') + self.assertFalse(self.base_resource._is_stale) # refresh() is called in the constructor self.conn.reset_mock() def test_refresh(self): self.base_resource.refresh() + self.conn.get.assert_not_called() + + def test_refresh_force(self): + self.base_resource.refresh(force=True) + self.conn.get.assert_called_once_with(path='/Foo') + + def test_invalidate(self): + self.base_resource.invalidate() + self.conn.get.assert_not_called() + + self.base_resource.refresh() + self.conn.get.assert_called_once_with(path='/Foo') + + def test_invalidate_force_refresh(self): + self.base_resource.invalidate(force_refresh=True) self.conn.get.assert_called_once_with(path='/Foo') @@ -187,37 +203,39 @@ class FieldTestCase(base.TestCase): def test_missing_required(self): del self.json['String'] - self.assertRaisesRegex(exceptions.MissingAttributeError, - 'String', self.test_resource.refresh) + self.assertRaisesRegex( + exceptions.MissingAttributeError, + 'String', self.test_resource.refresh, force=True) def test_missing_nested_required(self): del self.json['Nested']['String'] - self.assertRaisesRegex(exceptions.MissingAttributeError, - 'Nested/String', self.test_resource.refresh) + self.assertRaisesRegex( + exceptions.MissingAttributeError, + 'Nested/String', self.test_resource.refresh, force=True) def test_missing_nested_required2(self): del self.json['Nested']['Object']['Field'] self.assertRaisesRegex(exceptions.MissingAttributeError, 'Nested/Object/Field', - self.test_resource.refresh) + self.test_resource.refresh, force=True) def test_malformed_int(self): self.json['Integer'] = 'banana' self.assertRaisesRegex( exceptions.MalformedAttributeError, 'attribute Integer is malformed.*invalid literal for int', - self.test_resource.refresh) + self.test_resource.refresh, force=True) def test_malformed_nested_int(self): self.json['Nested']['Integer'] = 'banana' self.assertRaisesRegex( exceptions.MalformedAttributeError, 'attribute Nested/Integer is malformed.*invalid literal for int', - self.test_resource.refresh) + self.test_resource.refresh, force=True) def test_mapping_missing(self): self.json['Nested']['Mapped'] = 'banana' - self.test_resource.refresh() + self.test_resource.refresh(force=True) self.assertIsNone(self.test_resource.nested.mapped)