Merge pull request #230 from singnet/senna-223-2

[#223] Update log messages

CHANGELOG

@@ -1,2 +1,2 @@
 [#201] Implement fetch() in the DAS API
-[#223] Updates in docstrings
+[#223] Updates in docstrings and logging messages

hyperon_das/cache.py

@@ -491,7 +491,7 @@ def _filter(self, link: Dict[str, Any], targets: Dict[str, Any]) -> bool:
         if self.custom_filter and callable(self.custom_filter) and not self.targets_only:
             ret = self.custom_filter(link)
             if not isinstance(ret, bool):
-                raise TypeError('The function must return a boolean')
+                raise TypeError('Filter must return bool')
             if ret is False:
                 return False
 
@@ -556,7 +556,7 @@ def _filter(self, target: Dict[str, Any]) -> bool:
         if self.source.custom_filter and callable(self.source.custom_filter):
             ret = self.source.custom_filter(target)
             if not isinstance(ret, bool):
-                raise TypeError('The function must return a boolean')
+                raise TypeError('Filter must return bool')
             if ret is False:
                 return False
 

hyperon_das/client.py

@@ -7,13 +7,13 @@
 
 from hyperon_das.exceptions import ConnectionError, HTTPError, RequestError, TimeoutError
 from hyperon_das.logger import logger
-from hyperon_das.utils import connect_to_server, deserialize, serialize
+from hyperon_das.utils import connect_to_server, das_error, deserialize, serialize
 
 
 class FunctionsClient:
     def __init__(self, host: str, port: int, server_count: int = 0, name: Optional[str] = None):
-        if not host:
-            raise ValueError('Host is required')
+        if not host or not port:
+            das_error(ValueError("'host' and 'port' are mandatory parameters"))
 
         self.url = connect_to_server(host, port)
 
@@ -37,7 +37,7 @@ def _send_request(self, payload) -> Any:
             try:
                 response_data = deserialize(response.content)
             except pickle.UnpicklingError as e:
-                raise Exception(f"Unpickling error: {str(e)}")
+                das_error(Exception(f"Unpickling error: {str(e)}"))
 
             if response.status_code == 200:
                 return response_data
@@ -46,26 +46,34 @@ def _send_request(self, payload) -> Any:
                     'error', f'Unknown error with status code {response.status_code}'
                 )
         except exceptions.ConnectionError as e:
-            raise ConnectionError(
-                message=f"Connection error for URL: '{self.url}' with payload: '{payload}'",
-                details=str(e),
+            das_error(
+                ConnectionError(
+                    message=f"Connection error for URL: '{self.url}' with payload: '{payload}'",
+                    details=str(e),
+                )
             )
         except exceptions.Timeout as e:
-            raise TimeoutError(
-                message=f"Request timed out for URL: '{self.url}' with payload: '{payload}'",
-                details=str(e),
+            das_error(
+                TimeoutError(
+                    message=f"Request timed out for URL: '{self.url}' with payload: '{payload}'",
+                    details=str(e),
+                )
             )
         except exceptions.HTTPError as e:
             with contextlib.suppress(pickle.UnpicklingError):
                 return deserialize(response.content).get('error')
-            raise HTTPError(
-                message=f"HTTP error occurred for URL: '{self.url}' with payload: '{payload}'",
-                details=str(e),
+            das_error(
+                HTTPError(
+                    message=f"HTTP error for URL: '{self.url}' with payload: '{payload}'",
+                    details=str(e),
+                )
             )
         except exceptions.RequestException as e:
-            raise RequestError(
-                message=f"Request exception occurred for URL: '{self.url}' with payload: '{payload}'.",
-                details=str(e),
+            das_error(
+                RequestError(
+                    message=f"Request exception for URL: '{self.url}' with payload: '{payload}'.",
+                    details=str(e),
+                )
             )
 
     def get_atom(self, handle: str, **kwargs) -> Union[str, Dict]:
@@ -74,8 +82,8 @@ def get_atom(self, handle: str, **kwargs) -> Union[str, Dict]:
             'input': {'handle': handle},
         }
         response = self._send_request(payload)
-        if 'not exist' in response:
-            raise AtomDoesNotExist('error')
+        if 'Nonexistent' in response:
+            das_error(AtomDoesNotExist('error'))
         return response
 
     def get_node(self, node_type: str, node_name: str) -> Union[str, Dict]:
@@ -84,8 +92,8 @@ def get_node(self, node_type: str, node_name: str) -> Union[str, Dict]:
             'input': {'node_type': node_type, 'node_name': node_name},
         }
         response = self._send_request(payload)
-        if 'not exist' in response:
-            raise NodeDoesNotExist('error')
+        if 'Nonexistent' in response:
+            das_error(NodeDoesNotExist('error'))
         return response
 
     def get_link(self, link_type: str, link_targets: List[str]) -> Dict[str, Any]:
@@ -94,8 +102,8 @@ def get_link(self, link_type: str, link_targets: List[str]) -> Dict[str, Any]:
             'input': {'link_type': link_type, 'link_targets': link_targets},
         }
         response = self._send_request(payload)
-        if 'not exist' in response:
-            raise LinkDoesNotExist('error')
+        if 'Nonexistent' in response:
+            das_error(LinkDoesNotExist('error'))
         return response
 
     def get_links(

hyperon_das/query_engines.py

@@ -24,7 +24,7 @@
     UnexpectedQueryFormat,
 )
 from hyperon_das.logger import logger
-from hyperon_das.utils import Assignment, QueryAnswer
+from hyperon_das.utils import Assignment, QueryAnswer, das_error
 
 
 class QueryEngine(ABC):
@@ -99,10 +99,6 @@ def __init__(
         self.cache_manager: CacheManager = kwargs.get('cache_manager')
         self.local_backend = backend
 
-    def _error(self, exception: Exception):
-        logger().error(str(exception))
-        raise exception
-
     def _recursive_query(
         self,
         query: Union[Dict[str, Any], List[Dict[str, Any]]],
@@ -130,15 +126,15 @@ def _recursive_query(
                 elif target["atom_type"] == "variable":
                     matched_targets.append(ListIterator([QueryAnswer(target, None)]))
                 else:
-                    self._error(
+                    das_error(
                         UnexpectedQueryFormat(
                             message="Query processing reached an unexpected state",
                             details=f'link: {str(query)} link target: {str(query)}',
                         )
                     )
             return LazyQueryEvaluator(query["type"], matched_targets, self, parameters)
         else:
-            self._error(
+            das_error(
                 UnexpectedQueryFormat(
                     message="Query processing reached an unexpected state",
                     details=f'query: {str(query)}',
@@ -174,7 +170,11 @@ def _get_related_links(
         elif link_type != WILDCARD:
             return self.local_backend.get_matched_type(link_type, **kwargs)
         else:
-            self._error(ValueError("Invalid parameters"))
+            das_error(
+                ValueError(
+                    f"Invalid parameters. link_type = {link_type} target_types = {target_types} link_targets = {link_targets}"
+                )
+            )
 
     def _process_node(self, query: dict) -> List[dict]:
         try:
@@ -246,25 +246,25 @@ def _handle_to_atoms(self, handle: str) -> Union[List[dict], dict]:
     def get_atom(self, handle: str, **kwargs) -> Union[Dict[str, Any], None]:
         try:
             return self.local_backend.get_atom(handle, **kwargs)
-        except AtomDoesNotExist as e:
-            raise e
+        except AtomDoesNotExist as exception:
+            das_error(exception)
 
     def get_node(self, node_type: str, node_name: str) -> Union[Dict[str, Any], None]:
         try:
             node_handle = self.local_backend.node_handle(node_type, node_name)
             return self.local_backend.get_atom(node_handle)
         except AtomDoesNotExist:
-            raise NodeDoesNotExist(
-                message='This node does not exist', details=f'{node_type}:{node_name}'
+            das_error(
+                NodeDoesNotExist(message="Nonexistent node.", details=f'{node_type}:{node_name}')
             )
 
     def get_link(self, link_type: str, link_targets: List[str]) -> Union[Dict[str, Any], None]:
         try:
             link_handle = self.local_backend.link_handle(link_type, link_targets)
             return self.local_backend.get_atom(link_handle)
         except AtomDoesNotExist:
-            raise LinkDoesNotExist(
-                message='This link does not exist', details=f'{link_type}:{link_targets}'
+            das_error(
+                LinkDoesNotExist(message='Nonexistent link', details=f'{link_type}:{link_targets}')
             )
 
     def get_links(
@@ -377,7 +377,7 @@ def fetch(
             self.cache_manager.bulk_insert(documents)
         else:
             if 'atom_type' not in query:
-                raise ValueError('Invalid query: missing atom_type')
+                das_error(ValueError('Invalid query: missing atom_type'))
 
             atom_type = query['atom_type']
 
@@ -386,7 +386,9 @@ def fetch(
             elif atom_type == 'link':
                 return self._process_link(query)
             else:
-                raise ValueError('Invalid atom type')
+                das_error(
+                    ValueError("Invalid atom type: {atom_type}. Use 'node' or 'link' instead.")
+                )
 
 
 class RemoteQueryEngine(QueryEngine):
@@ -396,8 +398,8 @@ def __init__(self, backend, system_parameters: Dict[str, Any], kwargs: Optional[
         self.local_query_engine = LocalQueryEngine(backend, kwargs)
         self.host = kwargs.get('host')
         self.port = kwargs.get('port')
-        if not self.host:
-            raise InvalidDASParameters(message='Send `host` parameter to connect in a remote DAS')
+        if not self.host or not self.port:
+            das_error(InvalidDASParameters(message="'host' and 'port' are mandatory parameters"))
         self.remote_das = FunctionsClient(self.host, self.port)
 
     def get_atom(self, handle: str, **kwargs) -> Dict[str, Any]:
@@ -406,10 +408,8 @@ def get_atom(self, handle: str, **kwargs) -> Dict[str, Any]:
         except AtomDoesNotExist:
             try:
                 atom = self.remote_das.get_atom(handle, **kwargs)
-            except AtomDoesNotExist:
-                raise AtomDoesNotExist(
-                    message='This atom does not exist', details=f'handle:{handle}'
-                )
+            except AtomDoesNotExist as exception:
+                das_error(exception)
         return atom
 
     def get_node(self, node_type: str, node_name: str) -> Dict[str, Any]:
@@ -418,10 +418,8 @@ def get_node(self, node_type: str, node_name: str) -> Dict[str, Any]:
         except NodeDoesNotExist:
             try:
                 node = self.remote_das.get_node(node_type, node_name)
-            except NodeDoesNotExist:
-                raise NodeDoesNotExist(
-                    message='This node does not exist', details=f'{node_type}:{node_name}'
-                )
+            except NodeDoesNotExist as exception:
+                das_error(exception)
         return node
 
     def get_link(self, link_type: str, link_targets: List[str]) -> Dict[str, Any]:
@@ -430,10 +428,8 @@ def get_link(self, link_type: str, link_targets: List[str]) -> Dict[str, Any]:
         except LinkDoesNotExist:
             try:
                 link = self.remote_das.get_link(link_type, link_targets)
-            except LinkDoesNotExist:
-                raise LinkDoesNotExist(
-                    message='This link does not exist', details=f'{link_type}:{link_targets}'
-                )
+            except LinkDoesNotExist as exception:
+                das_error(exception)
         return link
 
     def get_links(
@@ -498,11 +494,16 @@ def query(
         elif query_scope == 'local_only':
             answer = self.local_query_engine.query(query, parameters)
         elif query_scope == 'local_and_remote':
-            # This type is not available yet
-            raise QueryParametersException
+            das_error(
+                QueryParametersException(
+                    message=f"Invalid value for parameter 'query_scope': '{query_scope}'. This type of query scope is not implemented yet"
+                )
+            )
         else:
-            raise QueryParametersException(
-                message=f'Invalid value for parameter "query_scope": "{query_scope}"'
+            das_error(
+                QueryParametersException(
+                    message=f'Invalid value for "query_scope": "{query_scope}"'
+                )
             )
         return answer
 
@@ -515,7 +516,7 @@ def commit(self):
         return self.remote_das.commit_changes()
 
     def reindex(self, pattern_index_templates: Optional[Dict[str, Dict[str, Any]]]):
-        raise NotImplementedError()
+        das_error(NotImplementedError())
 
     def create_field_index(
         self,

hyperon_das/utils.py

@@ -18,6 +18,11 @@
 from hyperon_das.logger import logger
 
 
+def das_error(exception: Exception):
+    logger().error(str(exception))
+    raise exception
+
+
 class Assignment:
     @staticmethod
     def compose(components: List["Assignment"]) -> Optional["Assignment"]:

tests/unit/test_das.py

@@ -40,7 +40,7 @@ def test_get_incoming_links(self):
         assert len(links) == 7
 
         with mock.patch('hyperon_das.utils.check_server_connection', return_value=(200, 'OK')):
-            das_remote = DistributedAtomSpaceMock('remote', host='test')
+            das_remote = DistributedAtomSpaceMock('remote', host='test', port=8080)
 
         with mock.patch(
             'hyperon_das.client.FunctionsClient.get_incoming_links', return_value=(0, [])

tests/unit/test_traverse_engine.py

@@ -352,7 +352,7 @@ def my_second_filter(link):
 
             with pytest.raises(TypeError) as exc:
                 _build_atom_answer(animal_base_handles.human, filter=my_second_filter)
-            assert exc.value.args[0] == 'The function must return a boolean'
+            assert exc.value.args[0] == 'Filter must return bool'
 
         def _mammal_links():
             answers = _build_atom_answer(animal_base_handles.mammal, link_type='Inheritance')