nih-sparc / sparc.client

Python client for NIH SPARC
https://docs.sparc.science/docs/sparc-python-client
Apache License 2.0
0 stars 8 forks source link

SparcClient without config file fails #40

Closed mhalle closed 3 weeks ago

mhalle commented 1 month ago

The most basic instantiation of a python client SparcClient without a config file fails:

In [3]: client = SparcClient()
---------------------------------------------------------------------------
KeyError                                  Traceback (most recent call last)
Cell In[3], line 1
----> 1 client = SparcClient()

File ~/.pyenv/versions/3.12.2/envs/sparc-3.12.2/lib/python3.12/site-packages/sparc/client/client.py:55, in SparcClient.__init__(self, config_file, connect)
     53     config.read(config_file)
     54     logging.debug(str(config))
---> 55 current_config = config["global"]["default_profile"]
     57 logging.debug("Using the following config:")
     58 logging.debug(str(config[current_config]))

File ~/.pyenv/versions/3.12.2/lib/python3.12/configparser.py:941, in RawConfigParser.__getitem__(self, key)
    939 def __getitem__(self, key):
    940     if key != self.default_section and not self.has_section(key):
--> 941         raise KeyError(key)
    942     return self._proxies[key]

    KeyError: 'global'
mhalle commented 1 month ago

A config file for a web client absolutely should not be required to get to public data that does not require login. It introduces significant friction for quick access from, say, a cloud jupyter notebook that may not be accessing the file system typically.

The documentation says a config file is required, and points to this page for a sample config: https://github.com/athril/sparc.client#readme

However, there's no config file there, and I can't find a way to generate one.

A config file should not be mandated. In memory store of configuration information is a better approach, with a way to read in and save the configuration information provided as a convenience.

nickerso commented 1 month ago

@athril - something to consider. We should definitely fix the docs to make sure a basic "ready to go" config file is available. But possibly also revisit the earlier design decision to require a config file?

muftring commented 1 month ago

Thanks @nickerso. I have posted this on our team Slack, in case the GitHub notification is not seen.

athril commented 1 month ago

Good point @mhalle! Thank you for bringing this, A fix is ready for the review @nickerso .