From 69dfcf91531a2b9db80b3e1ab9741047ec77a6df Mon Sep 17 00:00:00 2001 From: "David H. Irving" Date: Tue, 24 Oct 2023 13:43:58 -0700 Subject: [PATCH] Use FastAPI dependency overrides Instead of manually setting a global variable to configure the test server's Butler, use FastAPI's dependency injection framework. --- .../butler/remote_butler/server/__init__.py | 3 +- .../butler/remote_butler/server/_factory.py | 36 +++++++++++++++++++ .../butler/remote_butler/server/_server.py | 31 ++++++---------- tests/test_server.py | 22 ++++++------ 4 files changed, 58 insertions(+), 34 deletions(-) create mode 100644 python/lsst/daf/butler/remote_butler/server/_factory.py diff --git a/python/lsst/daf/butler/remote_butler/server/__init__.py b/python/lsst/daf/butler/remote_butler/server/__init__.py index 0377dc8a5d..a3fea4f9c3 100644 --- a/python/lsst/daf/butler/remote_butler/server/__init__.py +++ b/python/lsst/daf/butler/remote_butler/server/__init__.py @@ -25,4 +25,5 @@ # You should have received a copy of the GNU General Public License # along with this program. If not, see . -from ._server import app +from ._factory import Factory +from ._server import app, factory_dependency diff --git a/python/lsst/daf/butler/remote_butler/server/_factory.py b/python/lsst/daf/butler/remote_butler/server/_factory.py new file mode 100644 index 0000000000..da5ff1b1c9 --- /dev/null +++ b/python/lsst/daf/butler/remote_butler/server/_factory.py @@ -0,0 +1,36 @@ +# This file is part of daf_butler. +# +# Developed for the LSST Data Management System. +# This product includes software developed by the LSST Project +# (http://www.lsst.org). +# See the COPYRIGHT file at the top-level directory of this distribution +# for details of code ownership. +# +# This software is dual licensed under the GNU General Public License and also +# under a 3-clause BSD license. Recipients may choose which of these licenses +# to use; please see the files gpl-3.0.txt and/or bsd_license.txt, +# respectively. If you choose the GPL option then the following text applies +# (but note that there is still no warranty even if you opt for BSD instead): +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +from lsst.daf.butler import Butler + + +class Factory: + def __init__(self, *, butler: Butler): + self._butler = butler + + def create_butler(self) -> Butler: + return self._butler diff --git a/python/lsst/daf/butler/remote_butler/server/_server.py b/python/lsst/daf/butler/remote_butler/server/_server.py index 8170612085..8f7ad4b1d7 100644 --- a/python/lsst/daf/butler/remote_butler/server/_server.py +++ b/python/lsst/daf/butler/remote_butler/server/_server.py @@ -30,12 +30,15 @@ __all__ = () import logging +from functools import cache from typing import Any from fastapi import Depends, FastAPI from fastapi.middleware.gzip import GZipMiddleware from lsst.daf.butler import Butler +from ._factory import Factory + BUTLER_ROOT = "ci_hsc_gen3/DATA" log = logging.getLogger("excalibur") @@ -44,28 +47,13 @@ app.add_middleware(GZipMiddleware, minimum_size=1000) -GLOBAL_READWRITE_BUTLER: Butler | None = None -GLOBAL_READONLY_BUTLER: Butler | None = None - - -def _make_global_butler() -> None: - global GLOBAL_READONLY_BUTLER, GLOBAL_READWRITE_BUTLER - if GLOBAL_READONLY_BUTLER is None: - GLOBAL_READONLY_BUTLER = Butler.from_config(BUTLER_ROOT, writeable=False) - if GLOBAL_READWRITE_BUTLER is None: - GLOBAL_READWRITE_BUTLER = Butler.from_config(BUTLER_ROOT, writeable=True) - - -def butler_readonly_dependency() -> Butler: - """Return global read-only butler.""" - _make_global_butler() - return Butler.from_config(butler=GLOBAL_READONLY_BUTLER) +@cache +def _make_global_butler() -> Butler: + return Butler.from_config(BUTLER_ROOT) -def butler_readwrite_dependency() -> Butler: - """Return read-write butler.""" - _make_global_butler() - return Butler.from_config(butler=GLOBAL_READWRITE_BUTLER) +def factory_dependency() -> Factory: + return Factory(butler=_make_global_butler()) @app.get("/butler/") @@ -75,6 +63,7 @@ def read_root() -> str: @app.get("/butler/v1/universe", response_model=dict[str, Any]) -def get_dimension_universe(butler: Butler = Depends(butler_readonly_dependency)) -> dict[str, Any]: +def get_dimension_universe(factory: Factory = Depends(factory_dependency)) -> dict[str, Any]: """Allow remote client to get dimensions definition.""" + butler = factory.create_butler() return butler.dimensions.dimensionConfig.toDict() diff --git a/tests/test_server.py b/tests/test_server.py index f18a6a81c8..f2e157cb23 100644 --- a/tests/test_server.py +++ b/tests/test_server.py @@ -29,17 +29,15 @@ import unittest try: - import lsst.daf.butler.remote_butler.server._server - # Failing to import any of these should disable the tests. from fastapi.testclient import TestClient from lsst.daf.butler.remote_butler import RemoteButler - from lsst.daf.butler.remote_butler.server._server import app + from lsst.daf.butler.remote_butler.server import Factory, app, factory_dependency except ImportError: TestClient = None app = None -from lsst.daf.butler import CollectionType +from lsst.daf.butler import Butler from lsst.daf.butler.tests.utils import MetricTestRepo, makeTestTempDir, removeTestTempDir TESTDIR = os.path.abspath(os.path.dirname(__file__)) @@ -67,21 +65,21 @@ def setUpClass(cls): # First create a butler and populate it. cls.root = makeTestTempDir(TESTDIR) cls.repo = MetricTestRepo(root=cls.root, configFile=os.path.join(TESTDIR, "config/basic/butler.yaml")) + # Override the server's Butler initialization to point at our test repo + server_butler = Butler.from_config(cls.root) - # Add a collection chain. - cls.repo.butler.registry.registerCollection("chain", CollectionType.CHAINED) - cls.repo.butler.registry.setCollectionChain("chain", ["ingest"]) + def create_factory_dependency(): + return Factory(butler=server_butler) - # Globally change where the server thinks its butler repository - # is located. This will prevent any other server tests and is - # not a long term fix. - lsst.daf.butler.remote_butler.server._server.BUTLER_ROOT = cls.root - cls.client = TestClient(app) + app.dependency_overrides[factory_dependency] = create_factory_dependency + # Set up the RemoteButler that will connect to the server + cls.client = TestClient(app) cls.butler = _make_remote_butler(cls.client) @classmethod def tearDownClass(cls): + del app.dependency_overrides[factory_dependency] removeTestTempDir(cls.root) def test_simple(self):