corrected comments based on PR feedback

Signed-off-by: rherrell <russ.herrell@hpe.com>

Author: rherrell

sunfish_plugins/events_handlers/redfish/redfish_event_handler.py

@@ -46,8 +46,6 @@ def AggregationSourceDiscovered(cls, event_handler: EventHandlerInterface, event
         response = response.json()
 
         ### Save agent registration
-        # connection_method_name = connectionMethodId.split('/')[-1]
-        # connection_method_name = connectionMethodId[:-len(connection_method_name)]
         event_handler.core.storage_backend.write(response)
 
         aggregation_source_id = str(uuid.uuid4())
@@ -85,8 +83,6 @@ def ResourceCreated(cls, event_handler: EventHandlerInterface, event: dict, cont
         # sunfishAliasDB contains renaming data, the alias xref array, the boundaryLink 
         # data, and assorted flags that are used during upload renaming and final merge of 
         # boundary components based on boundary links.
-
-        #
         #
 
         logger.info("New resource created")
@@ -149,7 +145,6 @@ def TriggerEvent(cls, event_handler: EventHandlerInterface, event: dict, context
 		#
         logger.info("TriggerEvent method called")
         file_to_send = event['MessageArgs'][0]  # relative Resource Path
-        #file_path = os.path.join(self.conf['redfish_root'], file_to_send)
         hostname = event['MessageArgs'][1]  # target address
         destination = hostname + "/EventListener" # may match a Subscription object's 'Destination' property
         logger.debug(f"path of file_to_send is {file_to_send}")
@@ -299,7 +294,6 @@ def forward_event(self, list, payload):
         Returns:
             list: list of all the reachable subcribers for the event.
         """
-        # resp = 400
 
         for id in list:
             path = os.path.join(self.redfish_root, 'EventService', 'Subscriptions', id)
@@ -403,39 +397,24 @@ def handleNestedObject(self, obj):
         # this needs to be done on ALL agents, not just the one we just uploaded
         RedfishEventHandler.updateAllAgentsRedirectedLinks(self)
 
-        return visited  #why not the 'fetched' list?
+        return visited  
 
     def create_uploaded_object(self, path: str, payload: dict):
         # before to add the ID and to call the methods there should be the json validation
 
         # generate unique uuid if is not present
         if '@odata.id' not in payload and 'Id' not in payload:
             pass
-            #id = str(uuid.uuid4())
-            #to_add = {
-                #'Id': id,
-                #'@odata.id': os.path.join(path, id)
-            #}
-            #payload.update(to_add)
             raise exception(f"create_uploaded_object: no Redfish ID (@odata.id) found")
 
-        #object_type = self._get_type(payload)
         # we assume agents can upload collections, just not the root level collections
         # we will check for uploaded collections later
-        #if "Collection" in object_type:
-            #raise CollectionNotSupported()
 
         payload_to_write = payload
 
         try:
-            # 1. check the path target of the operation exists
-            # self.storage_backend.read(path)
-            # 2. we don't check the manager; we assume uploading agent is the manager unless it says otherwise
-            #agent_response = self.objects_manager.forward_to_manager(SunfishRequestType.CREATE, path, payload=payload)
-            #if agent_response:
-                #payload_to_write = agent_response
-            # 3. should be no custom handler, this is not a POST, we upload the objects directly into the Redfish database
-            #self.objects_handler.dispatch(object_type, path, SunfishRequestType.CREATE, payload=payload)
+            # this would be another location to verify new object to be written 
+            # meets Sunfish and Redfish requirements
             pass
         except ResourceNotFound:
             logger.error("The collection where the resource is to be created does not exist.")
@@ -445,7 +424,7 @@ def create_uploaded_object(self, path: str, payload: dict):
             # The object does not have a handler.
             logger.debug(f"The object {object_type} does not have a custom handler")
             pass
-        # 4. persist change in Sunfish tree
+        # persist change in Sunfish tree
         return self.storage_backend.write(payload_to_write)
 
     def get_aggregation_source(self, aggregation_source):
@@ -500,7 +479,10 @@ def fetchResource(self, obj_id, aggregation_source):
         if response.status_code == 200: # Agent must have returned this object
             redfish_obj = response.json()
             # however, it must be a minimally valid object
-            # the following test should really be more extensive, but for now:
+            # This would be a great spot to insert a call to a Redfish schema validation function
+            # that could return a grading of this new redfish_obj: [PASS, FAIL, CAUTIONS] 
+            # However, we are debugging not just code, but also new Redfish schema,
+            # so for now we just test for two required Redfish Properties to help weed out obviously incorrect responses
             if '@odata.id' in redfish_obj and '@odata.type' in redfish_obj:
 
                 # now rename if necessary and copy object into Sunfish inventory
@@ -608,7 +590,6 @@ def createInspectedObject(self,redfish_obj, aggregation_source):
                 if redfish_obj["Oem"]["Sunfish_RM"]["BoundaryComponent"] == "BoundaryPort":
                     RedfishEventHandler.track_boundary_port(self, redfish_obj, aggregation_source)
                 # is this new object a new fabric object with same fabric UUID as an existing fabric?
-                # RedfishEventHandler.checkForAliasedFabrics(self, redfish_obj, aggregation_source)
                 RedfishEventHandler.create_uploaded_object(self, file_path, redfish_obj)
 
         return redfish_obj
@@ -863,7 +844,6 @@ def redirectUpstreamPortLinks(self,owning_agent_id, agent_bp_obj,uri_aliasDB):
         # extract the Endpoint URI associated with this parent object
         host_obj = self.storage_backend.read(host_link)
         redirected_endpoint = host_obj["Links"]["Endpoints"][0]["@odata.id"]
-        #redirected_endpoint = "None"  #for now, to test
 
         if "Links" not in agent_bp_obj:
             agent_bp_obj["Links"] = {}

sunfish_plugins/storage/file_system_backend/backend_FS.py

@@ -68,25 +68,32 @@ def write(self, payload: dict):
         parent_is_collection = True # default assumption
         last_parent_to_exist=""
 
-        print(f"BackendFS.write called on {id}")
+        logging.info(f"BackendFS.write called on {id}")
         id = id.split('/')
         for index in range(2, len(id[1:])):
             to_check = os.path.join('/'.join(id[:index]), 'index.json')
             to_check = os.path.join(os.getcwd(), self.root, to_check)
-            print(f"BackendFS.write():  path to check: {to_check}")
+            logging.info(f"BackendFS.write():  path to check: {to_check}")
             if os.path.exists(to_check) is True:
                 # capture this parent path as existing
                 last_parent_to_exist = to_check
             if os.path.exists(to_check) is False:
-                print("path does not exist\n")
+                logging.info("path does not exist\n")
                 # nice to know, but NOT an error!
-                #raise ActionNotAllowed()
+                # Log the situation and continue 
+        
+
+
+        # This particular code block looks unfinished and its purpose/functionality is unknown.
+        # It looks as if part of this block was intended to fill in missing path elements and is redundant
+        # with code just below this block. This block also sets a flag that is never used.  - more analysis required.
+        # 
         '''
             with open(to_check, 'r') as data_json:
                 data = json.load(data_json)
                 data_json.close()
                 if 'Collection' in data["@odata.type"]:
-                    print("path is to a Collection\n")
+                    logging.info("path is to a Collection\n")
                     members = data["Members"]
                     for x in members:
                         if x["@odata.id"] == os.path.join(self.redfish_root, '/'.join(id[:index + 1])):
@@ -101,7 +108,7 @@ def write(self, payload: dict):
                                 present = True
                             else:
                                 el["@odata.id"] = os.path.join(self.redfish_root, '/'.join(id[:index + 1]))
-                                print(f"BackendFS.write of {el['@odata.id']}")
+                                logging.info(f"BackendFS.write of {el['@odata.id']}")
                                 with open(to_check, 'w') as data_json:
                                     json.dump(data, data_json, indent=4, sort_keys=True)
                                     data_json.close()
@@ -117,7 +124,6 @@ def write(self, payload: dict):
             for i in range(0, last_element - 1):
                 full_collection = full_collection + id[i] + '/'
 
-        #collection_type = os.path.join(full_collection, collection_type)
         full_collection = os.path.join(full_collection, collection_type)
 
         collection_path = os.path.join(os.getcwd(), self.root,
@@ -128,23 +134,13 @@ def write(self, payload: dict):
         # check if the directory of the Collection already exists
         if not os.path.exists(collection_path):
             # if parent directory doesn't exist, we assume it is a collection and create the collection
-            print(f"backendFS.write: making collection path directory")
+            logging.info(f"backendFS.write: making collection path directory")
             os.makedirs(collection_path)
 
             # the following line assumes the path element name dictates the collection type
             # it is more proper to examine the @odata.type property of the object being created!
             config = utils.generate_collection(collection_type)
 
-            # if the item to be written is managed by an agent, we want the collection containing it to also be marked
-            # accordingly. We do this only for collections to be created because we assume that if the collection is
-            # there already:
-            #  a. The collection is a first level one that is managed by Sunfish
-            #  b. The collection was previously created during an agent discovery process and therefore already marked
-            # if "Oem" in payload and "Sunfish_RM" in payload["Oem"] and len(id) > 2 :
-            #     if "Oem" not in config:
-            #         config["Oem"] = {}
-            #     config["Oem"]["Sunfish_RM"] = payload["Oem"]["Sunfish_RM"]
-
             ## write file Resources/[folder]/index.json
             with open(os.path.join(collection_path, "index.json"), "w") as fd:
                 fd.write(json.dumps(config, indent=4, sort_keys=True))
@@ -164,13 +160,13 @@ def write(self, payload: dict):
                 parent_data = json.load(data_json)
                 data_json.close()
             if 'Collection' in parent_data["@odata.type"]:
-                print("parent path is to a Collection\n")
+                logging.info("parent path is to a Collection\n")
                 if utils.check_unique_id(index_path, payload['@odata.id']) is False:
                     raise AlreadyExists(payload['@odata.id'])
                     pass
             else:
-                print("path is to an object\n")
-                parent_is_collection = False  #
+                logging.info("path is to an object\n")
+                parent_is_collection = False  
                 pass
 
 
@@ -240,7 +236,7 @@ def _update_object(self, payload: dict, replace: bool):
         Returns:
             str: id of the updated resource
         """
-        ## code that re-write into file
+        # code that re-write into file
         logging.info('BackendFS patch update called')
 
         # get ID and collection from payload
@@ -280,7 +276,6 @@ def _update_object(self, payload: dict, replace: bool):
             raise ResourceNotFound(resource_id)
 
         result: str = self.read(payload["@odata.id"])
-        # result:str = payload['@odata.id']
 
         return result
 
@@ -297,7 +292,7 @@ def remove(self, path:str):
         Returns:
             str: confirmation string
         """
-        ## code that removes a file
+        # code that removes a file
         logging.info('BackendFS: remove called')
 
         length = len(self.redfish_root)