[#201] Implement fetch() in the DAS API #207
Conversation
hyperon_das/das.py
Outdated
| das = DistributedAtomSpace() | ||
| das.fetch(query, host='123.4.5.6', port=8080) | ||
| """ | ||
| if not kwargs.get('running_on_server') and not host and not port: |
There was a problem hiding this comment.
This check seems to be wrong. If it's not running on server then it's running in a local DAS. But if it's running in a local DAS, fetch will need both host and port. However, your check will not raise if one of them is None but the other is OK.
hyperon_das/das.py
Outdated
| das = DistributedAtomSpace() | ||
| das.fetch(query, host='123.4.5.6', port=8080) | ||
| """ | ||
| if not kwargs.get('running_on_server') and not host and not port: |
There was a problem hiding this comment.
You shouldn't need to have a KW parameter to check if it's a local or a remote DAS. There should be a more elegant way to identify if it's running on a local or a remote DAS. This is a method in DAS public API so you are expecting that the user will pass this parameter... This seems like bad UX.
hyperon_das/das.py
Outdated
|
|
||
| if atomdb_parameter == "ram": | ||
| def __init__(self, system_parameters: Dict[str, Any] = {}, **kwargs) -> None: | ||
| if not system_parameters.get('running_on_server'): |
There was a problem hiding this comment.
Please make an explicit private method _set_default_system_parameters() to setup default parameters as we'll probably have other cases where we'll want to do the same. I believe the algorithm should be as you did:
def __init__(self, system_parameters...):
...
self.system_parameters = system_parameters
self._set_default_system_parameters()
...
def _set_default_system_parameters():
if not system_parameters.get('running_on_server'):
system_parameters['running_on_server'] = False
There was a problem hiding this comment.
An alternative design would be to use a dataclass SystemParameters instead of a dict. This way defaults would be put in the dataclass' constructor. This option has the advantage ensuring the use of the right parameter name across the code, which otherwise would be detected only in runtime. I'll let you decide.
Resolves #201