Add "local" config files to allow default config to be the same for e…

…verybody (c3-time-domain#71)

.github/workflows/run-tests.yml

@@ -16,6 +16,10 @@ jobs:
       COMPOSE_FILE: tests/docker-compose.yaml
 
     steps:
+      - name: Dump docker logs on failure
+        if: failure()
+        uses: jwalton/gh-docker-logs@v2
+
       - name: checkout code
         uses: actions/checkout@v3
         with:

.gitignore

@@ -1,4 +1,10 @@
 # some specific files we don't want in the repo
+local_config.yaml
+local_overrides.yaml
+local_augments.yaml
+tests/local_config.yaml
+tests/local_overrides.yaml
+tests/local_augments.yaml
 data/DECam_examples/c4d_221104_074232_ori.fits.fz
 
 # Byte-compiled / optimized / DLL files

README.md

@@ -130,12 +130,12 @@ Database migrations are handled with alembic.
 
 If you've just created a database and want to initialize it with all the tables, run
 ```
-  SEECHANGE_CONFIG=<configfile> alembic upgrade head
+  alembic upgrade head
 ```
 
 After editing any schema, you have to create new database migrations to apply them.  Do this by running something like:
 ```
-  SEECHANGE_CONFIG=<configfile> alembic revision --autogenerate -m "<put a short comment here>"
+  alembic revision --autogenerate -m "<put a short comment here>"
 ```
 The comment will go in the filename, so it should really be short.  
 Look out for any warnings, and review the created migration file before applying it (with `alembic upgrade head`).

alembic/env.py

@@ -1,3 +1,5 @@
+import os
+import pathlib
 from logging.config import fileConfig
 
 from sqlalchemy import engine_from_config
@@ -16,9 +18,11 @@
 if config.config_file_name is not None:
     fileConfig(config.config_file_name)
 
-from models import *
+default_config_file = str((pathlib.Path(__file__).parent.parent / 'default_config.yaml').resolve())
 import util.config
-util.config.Config.init()
+util.config.Config.init(os.getenv('SEECHANGE_CONFIG', default_config_file))
+
+from models import *
 
 # add your model's MetaData object here
 # for 'autogenerate' support

default_config.yaml

@@ -1,8 +1,12 @@
+overrides:
+  - local_overrides.yaml
+augments:
+  - local_augments.yaml
+
 path:
   data_root: null
   data_temp: null
-  server_data: null
-  # TODO: need to add additional options for server communications
+
 db:
   engine: postgresql
   user: postgres

devshell/docker-compose.yaml

@@ -16,8 +16,6 @@ services:
   seechange_dbmigrate:
     image: gchr.io/${GITHUB_REPOSITORY_OWNER}/seechange
     build: ../docker/application
-    environment:
-      SEECHANGE_CONFIG: /seechange/devshell/seechange_devshell.yaml
     depends_on:
       seechange_postgres:
          condition: service_healthy
@@ -32,8 +30,6 @@ services:
 seechange:
     image: gchr.io/${GITHUB_REPOSITORY_OWNER}/seechange
     build: ../docker/application
-    environment:
-      SEECHANGE_CONFIG: /seechange/devshell/seechange_devshell.yaml
     depends_on:
       seechange_postgres:
          condition: service_healthy

models/base.py

@@ -237,8 +237,21 @@ def recursive_merge(self, session, done_list=None):
 
 Base = declarative_base(cls=SeeChangeBase)
 
+ARCHIVE = None
 
-class FileOnDiskMixin:
+
+def get_archive_object():
+    """Return a global archive object. If it doesn't exist, create it based on the current config. """
+    global ARCHIVE
+    if ARCHIVE is None:
+        cfg = config.Config.get()
+        archive_specs = cfg.value('archive', None)
+        if archive_specs is not None:
+            ARCHIVE = Archive(**archive_specs)
+    return ARCHIVE
+
+
+class FileOnDiskMixin():
     """Mixin for objects that refer to files on disk.
 
     Files are assumed to live on the local disk (underneath the
@@ -315,15 +328,6 @@ class variables "local_path" and "archive" that are initialized from
     if not os.path.isdir(local_path):
         os.makedirs(local_path, exist_ok=True)
 
-    archive = cfg.value('archive', None)
-    if archive is not None:
-        archive = Archive( archive_url=cfg.value('archive.url'),
-                           path_base=cfg.value('archive.path_base'),
-                           token=cfg.value('archive.token'),
-                           verify_cert=cfg.value('archive.verify_cert'),
-                           local_read_dir=cfg.value('archive.read_dir'),
-                           local_write_dir=cfg.value('archive.write_dir') )
-
     @classmethod
     def safe_mkdir(cls, path):
         if path is None or path == '':
@@ -426,9 +430,22 @@ def __init__(self, *args, **kwargs):
         self.filepath = kwargs.pop('filepath', self.filepath)
         self.nofile = kwargs.pop('nofile', self._do_not_require_file_to_exist())
 
+        self._archive = None
+
     @orm.reconstructor
     def init_on_load(self):
         self.nofile = self._do_not_require_file_to_exist()
+        self._archive = None
+
+    @property
+    def archive(self):
+        if getattr(self, '_archive', None) is None:
+            self._archive = get_archive_object()
+        return self._archive
+
+    @archive.setter
+    def archive(self, value):
+        self._archive = value
 
     @staticmethod
     def _do_not_require_file_to_exist():

tests/conftest.py

@@ -3,13 +3,15 @@
 import uuid
 import wget
 import shutil
+import pathlib
 
 import numpy as np
 
 import sqlalchemy as sa
 
 from astropy.time import Time
 
+from util.config import Config
 from models.base import SmartSession, CODE_ROOT
 from models.provenance import CodeVersion, Provenance
 from models.exposure import Exposure
@@ -18,10 +20,33 @@
 from util import config
 from util.archive import Archive
 
+
+# idea taken from: https://shay-palachy.medium.com/temp-environment-variables-for-pytest-7253230bd777
+# this fixture should be the first thing loaded by the test suite
+@pytest.fixture(scope="session", autouse=True)
+def tests_setup_and_teardown():
+    # Will be executed before the first test
+    # print('Initial setup fixture loaded! ')
+
+    # make sure to load the test config
+    test_config_file = str((pathlib.Path(__file__).parent.parent / 'tests' / 'seechange_config_test.yaml').resolve())
+
+    Config.get(configfile=test_config_file, setdefault=True)
+
+    yield
+    # Will be executed after the last test
+    # print('Final teardown fixture executed! ')
+
+
 def rnd_str(n):
     return ''.join(np.random.choice(list('abcdefghijklmnopqrstuvwxyz'), n))
 
 
+@pytest.fixture
+def config_test():
+    return Config.get()
+
+
 @pytest.fixture(scope="session", autouse=True)
 def code_version():
     with SmartSession() as session:
@@ -271,29 +296,19 @@ def reference_entry(exposure_factory, provenance_base, provenance_extra):
 @pytest.fixture
 def archive():
     cfg = config.Config.get()
-    if cfg.value('archive') is None:
+    archive_specs = cfg.value('archive')
+    if archive_specs is None:
         raise ValueError( "archive in config is None" )
-    archive = Archive( archive_url=cfg.value('archive.url'),
-                       verify_cert=cfg.value('archive.verify_cert'),
-                       path_base=cfg.value('archive.path_base'),
-                       local_read_dir=cfg.value('archive.read_dir'),
-                       local_write_dir=cfg.value('archive.write_dir'),
-                       token=cfg.value('archive.token')
-                      )
+    archive = Archive( **archive_specs )
     yield archive
 
     # To tear down, we need to blow away the archive server's directory.
     # For the test suite, we've also mounted that directory locally, so
     # we can do that
-    archivebase = f"{os.getenv('ARCHIVE_DIR')}/{cfg.value('archive.path_base')}"
+    archivebase = f"{os.getenv('SEECHANGE_TEST_ARCHIVE_DIR')}/{cfg.value('archive.path_base')}"
     try:
         shutil.rmtree( archivebase )
     except FileNotFoundError:
         pass
 
-@pytest.fixture
-def config_test():
-    # Make sure the environment is set as expected for tests
-    assert os.getenv( "SEECHANGE_CONFIG" ) == "/seechange/tests/seechange_config_test.yaml"
-    return config.Config.get( os.getenv("SEECHANGE_CONFIG") )
 

tests/docker-compose.yaml

@@ -78,7 +78,7 @@ services:
       context: ../docker/application
     environment:
       SEECHANGE_CONFIG: /seechange/tests/seechange_config_test.yaml
-      ARCHIVE_DIR: /archive_storage/base
+      SEECHANGE_TEST_ARCHIVE_DIR: /archive_storage/base
     depends_on:
       setuptables:
         condition: service_completed_successfully
@@ -101,7 +101,7 @@ services:
       context: ../docker/application
     environment:
       SEECHANGE_CONFIG: /seechange/tests/seechange_config_test.yaml
-      ARCHIVE_DIR: /archive_storage/base
+      SEECHANGE_TEST_ARCHIVE_DIR: /archive_storage/base
     depends_on:
       setuptables:
         condition: service_completed_successfully

tests/models/test_base.py

@@ -130,7 +130,7 @@ def test_fileondisk_save_failuremodes( diskfile ):
 
 def test_fileondisk_save_singlefile( diskfile, archive ):
     cfg = config.Config.get()
-    archivebase = f"{os.getenv('ARCHIVE_DIR')}/{cfg.value('archive.path_base')}"
+    archivebase = f"{os.getenv('SEECHANGE_TEST_ARCHIVE_DIR')}/{cfg.value('archive.path_base')}"
 
     diskfile.filepath = 'test_fileondisk_save.dat'
     data1 = numpy.random.rand( 32 ).tobytes()
@@ -256,7 +256,7 @@ def test_fileondisk_save_singlefile( diskfile, archive ):
 def test_fileondisk_save_multifile( diskfile, archive ):
     try:
         cfg = config.Config.get()
-        archivebase = f"{os.getenv('ARCHIVE_DIR')}/{cfg.value('archive.path_base')}"
+        archivebase = f"{os.getenv('SEECHANGE_TEST_ARCHIVE_DIR')}/{cfg.value('archive.path_base')}"
 
         diskfile.filepath = 'test_fileondisk_save'
         data1 = numpy.random.rand( 32 ).tobytes()

tests/models/test_image.py

@@ -92,7 +92,7 @@ def test_image_archive_singlefile(demo_image, provenance_base, archive):
     demo_image.flags = np.random.randint(0, 100, size=demo_image.raw_data.shape, dtype=np.uint16)
 
     cfg = config.Config.get()
-    archivebase = f"{os.getenv('ARCHIVE_DIR')}/{cfg.value('archive.path_base')}"
+    archivebase = f"{os.getenv('SEECHANGE_TEST_ARCHIVE_DIR')}/{cfg.value('archive.path_base')}"
     single_fileness = cfg.value( 'storage.images.single_file' )
 
     try:
@@ -159,7 +159,7 @@ def test_image_archive_multifile(demo_image, provenance_base, archive):
     demo_image.flags = np.random.randint(0, 100, size=demo_image.raw_data.shape, dtype=np.uint16)
 
     cfg = config.Config.get()
-    archivebase = f"{os.getenv('ARCHIVE_DIR')}/{cfg.value('archive.path_base')}"
+    archivebase = f"{os.getenv('SEECHANGE_TEST_ARCHIVE_DIR')}/{cfg.value('archive.path_base')}"
     single_fileness = cfg.value( 'storage.images.single_file' )
 
     try:

tests/seechange_config_test.yaml

@@ -1,15 +1,19 @@
 preloads:
   - ../default_config.yaml
+overrides:
+  - local_overrides.yaml
+augments:
+  - local_augments.yaml
 
 db:
   host: seechange_postgres
 
 archive:
-  url: http://archive:8080/
+  archive_url: http://archive:8080/
   verify_cert: false
   path_base: test/
-  read_dir: null
-  write_dir: null
+  local_read_dir: null
+  local_write_dir: null
   token: insecure
 
 subtraction:

tests/util/test_config.py

@@ -15,25 +15,20 @@
 
 
 class TestConfig:
-    # Nominally, this tests should pass even if SEECHANGE_CONFIG is not None.  However, because there
-    # are autouse fixtures in ../conftest.py, one of the side effects is that the config default
-    # gets set.
-    @pytest.mark.skipif( os.getenv("SEECHANGE_CONFIG") is not None, reason="Clear SEECHANGE_CONFIG to test this" )
-    def test_no_default( self ):
-        assert config.Config._default == None
-
-    @pytest.mark.skipif( os.getenv("SEECHANGE_CONFIG") is None, reason="Set SEECHANGE_CONFIG to test this" )
-    def test_default_default( self ):
-        cfg = config.Config.get()
-        assert cfg._path == pathlib.Path( os.getenv("SEECHANGE_CONFIG") )
-
-    def test_set_default( self ):
-        cfg = config.Config.get( _rundir / 'test.yaml', setdefault=True )
-        assert config.Config._default == f'{ (_rundir / "test.yaml").resolve() }'
-
     @pytest.fixture(scope='class')
-    def cfg( self ):
-        return config.Config.get( _rundir / 'test.yaml', setdefault=True )
+    def cfg(self):
+        # print('setting up a config object with a spoof yaml file just for testing the config mechanism. ')
+        return config.Config.get(_rundir / 'test.yaml', setdefault=False)  # retain the default for other tests
+
+    def test_default_default( self ):
+        # make sure that when we load a config without parameters,
+        # it uses the default config file
+        default_config_path = (_rundir.parent.parent / 'default_config.yaml').resolve()
+        default_config_path = os.getenv('SEECHANGE_CONFIG', default_config_path)
+        assert config.Config._default_default == str(default_config_path)
+
+    def test_config_path( self, cfg ):
+        assert cfg._path == (_rundir / "test.yaml").resolve()
 
     def test_preload(self, cfg):
         assert cfg.value('preload1dict1.preload1_1val1') == '2_1val1'