feat: shortcut when running in AWS lambda; read config values from env

sg-s commented 5 months ago

problem

when running in lambda, reading a config file fails for some unknown reason.

solution

we shouldn't be reading a config file anyway, introduced a shortcut to read from envs directly

jonrkarr commented 5 months ago

Why is this necessary? The configuration module already reads from environment variables plus the default values (and user configuration files when they exist).

sg-s commented 5 months ago

@jonrkarr i am equally surprised.

however, it looks like confuse is failing at reading a config file on a read-only system, for reasons i don't understand:

https://deeporigin.slack.com/archives/C06986CSCRJ/p1718215645735529?thread_ts=1718215522.820199&cid=C06986CSCRJ

secondly, i have verified that this workaround allows this to run on AWS

sg-s commented 5 months ago

@jonrkarr specifically this line is failing:

value = confuse.Configuration("deep_origin", __name__)

relevant stack trace here:

File /var/lang/lib/python3.12/site-packages/deeporigin/utils.py:89, in _nucleus_url()
     85 @beartype
     86 def _nucleus_url() -> str:
     87     """returns URL for nucleus API endpoint"""
     88     return urljoin(
---> 89         get_value()["api_endpoint"],
     90         get_value()["nucleus_api_route"],
     91     )

File /var/lang/lib/python3.12/site-packages/deeporigin/config/__init__.py:37, in get_value(user_config_filenames)
     22 @functools.cache
     23 def get_value(
     24     user_config_filenames: collections.abc.Iterable[str] = (
   (...)
     27     ),
     28 ) -> confuse.templates.AttrDict:
     29     """Get the configuration for the Deep Origin CLI
     30 
     31     Args:
   (...)
     35         :obj:`confuse.templates.AttrDict`: configuration for the Deep Origin CLI
     36     """
---> 37     value = confuse.Configuration("deep_origin", __name__)
     39     # read the default configuration
     40     default_config_filename = os.path.join(CONFIG_DIR, "default.yml")

File /var/lang/lib/python3.12/site-packages/confuse/core.py:500, in Configuration.__init__(self, appname, modname, read, loader)
    497 self._env_var = '{0}DIR'.format(self.appname.upper())
    499 if read:
--> 500     self.read()

File /var/lang/lib/python3.12/site-packages/confuse/core.py:535, in Configuration.read(self, user, defaults)
    529 """Find and read the files for this configuration and set them
    530 as the sources for this configuration. To disable either
    531 discovered user configuration files or the in-package defaults,
    532 set `user` or `defaults` to `False`.
    533 """
    534 if user:
--> 535     self._add_user_source()
    536 if defaults:
    537     self._add_default_source()

File /var/lang/lib/python3.12/site-packages/confuse/core.py:514, in Configuration._add_user_source(self)
    509 def _add_user_source(self):
    510     """Add the configuration options from the YAML file in the
    511     user's configuration directory (given by `config_dir`) if it
    512     exists.
    513     """
--> 514     filename = self.user_config_path()
    515     self.add(YamlSource(filename, loader=self.loader, optional=True))

File /var/lang/lib/python3.12/site-packages/confuse/core.py:507, in Configuration.user_config_path(self)
    502 def user_config_path(self):
    503     """Points to the location of the user configuration.
    504 
    505     The file may not exist.
    506     """
--> 507     return os.path.join(self.config_dir(), CONFIG_FILENAME)

File /var/lang/lib/python3.12/site-packages/confuse/core.py:572, in Configuration.config_dir(self)
    570 # Ensure that the directory exists.
    571 try:
--> 572     os.makedirs(appdir)
    573 except OSError as e:
    574     if e.errno != errno.EEXIST:

File <frozen os>:215, in makedirs(name, mode, exist_ok)

File <frozen os>:215, in makedirs(name, mode, exist_ok)

File <frozen os>:225, in makedirs(name, mode, exist_ok)

OSError: [Errno 30] Read-only file system: '/home/sbx_user1051'

i've confirmed this occurs on local too. i agree that this PR implements a weird thing, so would love to not do this, but i don't see a way

sg-s commented 5 months ago

the suggest fix by confuse (to set user=False) is something we don't want, i think, because we want confuse to read from user configs

sg-s commented 5 months ago

@jonrkarr i have an even better minimal example. running deeporigin config in a read-only filesystem causes the same error:

deeporigin config

Traceback (most recent call last):
  File "/var/lang/bin/deeporigin", line 8, in <module>
    sys.exit(main())
             ^^^^^^
  File "/var/lang/lib/python3.12/site-packages/deeporigin/cli/__init__.py", line 117, in main
    app.run()
  File "/var/lang/lib/python3.12/site-packages/cement/core/foundation.py", line 948, in run
    return_val = self.controller._dispatch()
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/var/lang/lib/python3.12/site-packages/cement/ext/ext_argparse.py", line 810, in _dispatch
    return func()
           ^^^^^^
  File "/var/lang/lib/python3.12/site-packages/deeporigin/cli/__init__.py", line 69, in config
    data = get_value()
           ^^^^^^^^^^^
  File "/var/lang/lib/python3.12/site-packages/deeporigin/config/__init__.py", line 37, in get_value
    value = confuse.Configuration("deep_origin", __name__)
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/var/lang/lib/python3.12/site-packages/confuse/core.py", line 500, in __init__
    self.read()
  File "/var/lang/lib/python3.12/site-packages/confuse/core.py", line 535, in read
    self._add_user_source()
  File "/var/lang/lib/python3.12/site-packages/confuse/core.py", line 514, in _add_user_source
    filename = self.user_config_path()
               ^^^^^^^^^^^^^^^^^^^^^^^
  File "/var/lang/lib/python3.12/site-packages/confuse/core.py", line 507, in user_config_path
    return os.path.join(self.config_dir(), CONFIG_FILENAME)
                        ^^^^^^^^^^^^^^^^^
  File "/var/lang/lib/python3.12/site-packages/confuse/core.py", line 572, in config_dir
    os.makedirs(appdir)
  File "<frozen os>", line 215, in makedirs
  File "<frozen os>", line 225, in makedirs
OSError: [Errno 30] Read-only file system: '/root/.config'

sg-s commented 5 months ago

so confuse is trying to write something to disk when you ask it to just read the config

sg-s commented 5 months ago

this bit of code is to blame:

https://github.com/beetbox/confuse/blob/5001e185ade4a27c613d35904147af19931690e8/confuse/core.py#L573

sg-s commented 5 months ago

found the underlying issue. on *nix like systems, confuse assumes that ~/.config is writeable or creatable:

https://github.com/beetbox/confuse/blob/5001e185ade4a27c613d35904147af19931690e8/confuse/util.py#L9

this is not true on the lambda

jonrkarr commented 5 months ago

Interesting. Its sort of a bug. makedirs isn't necessary; confuse could be implemented differently.

Two things about the new code are weird to me

The if condition focuses on whether the environment is a lambda function, rather than the core issue whether the configuration files are writable
The if condition returns a different data structure

One more reusable way to do this is to read from /tmp, which is writable in lambda. This would avoid lambda-specific code and enable get_config to always return the same data structure.

If the user home directory isn't writable, read from /tmp instead
If /tmp isn't writable, let confuse raise an error

deeporiginbio / deeporigin-client

feat: shortcut when running in AWS lambda; read config values from env #43

problem

solution