Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DM-41367: Make Butler server deployable #901

Merged
merged 5 commits into from
Nov 3, 2023
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions python/lsst/daf/butler/remote_butler/_remote_butler.py
Original file line number Diff line number Diff line change
@@ -33,6 +33,7 @@

import httpx
from lsst.daf.butler import __version__
from lsst.daf.butler.repo_relocation import replaceRoot
from lsst.resources import ResourcePath, ResourcePathExpression
from lsst.utils.introspection import get_full_type_name

@@ -72,6 +73,16 @@ def __init__(
**kwargs: Any,
):
butler_config = ButlerConfig(config, searchPaths, without_datastore=True)
# There is a convention in Butler config files where <butlerRoot> in a
# configuration option refers to the directory containing the
# configuration file. We allow this for the remote butler's URL so
# that the server doesn't have to know which hostname it is being
# accessed from
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
# accessed from
# accessed from.

server_url_key = ("remote_butler", "url")
if server_url_key in butler_config:
butler_config[server_url_key] = replaceRoot(
butler_config[server_url_key], butler_config.configDir
)
self._config = RemoteButlerConfigModel.model_validate(butler_config)
self._dimensions: DimensionUniverse | None = None
# TODO: RegistryDefaults should have finish() called on it, but this
24 changes: 24 additions & 0 deletions python/lsst/daf/butler/remote_butler/server/_server.py
Original file line number Diff line number Diff line change
@@ -118,6 +118,30 @@ async def get_index() -> Metadata:
return get_metadata(package_name="lsst.daf.butler", application_name="butler")


@app.get(
"/butler/butler.yaml",
description=(
"Returns a Butler YAML configuration file that can be used to instantiate a Butler client"
" pointing at this server"
),
summary="Client configuration file",
response_model=dict[str, Any],
)
@app.get(
"/butler/butler.json",
description=(
"Returns a Butler JSON configuration file that can be used to instantiate a Butler client"
" pointing at this server"
),
summary="Client configuration file",
response_model=dict[str, Any],
)
async def get_client_config() -> dict[str, Any]:
# We can return JSON data for both the YAML and JSON case because all JSON
# files are parseable as YAML.
return {"cls": "lsst.daf.butler.remote_butler.RemoteButler", "remote_butler": {"url": "<butlerRoot>"}}


@app.get("/butler/v1/universe", response_model=dict[str, Any])
def get_dimension_universe(factory: Factory = Depends(factory_dependency)) -> dict[str, Any]:
"""Allow remote client to get dimensions definition."""
26 changes: 26 additions & 0 deletions tests/test_server.py
Original file line number Diff line number Diff line change
@@ -38,9 +38,12 @@
TestClient = None
app = None

from unittest.mock import patch

from lsst.daf.butler import Butler, DataCoordinate, DatasetRef, MissingDatasetTypeError, StorageClassFactory
from lsst.daf.butler.tests import DatastoreMock
from lsst.daf.butler.tests.utils import MetricTestRepo, makeTestTempDir, removeTestTempDir
from lsst.resources.http import HttpResourcePath

TESTDIR = os.path.abspath(os.path.dirname(__file__))

@@ -169,6 +172,29 @@ def test_find_dataset(self):
self.assertIsNone(self.butler.get_dataset(uuid.uuid4()))
self.assertIsNone(self.butler.get_dataset(uuid.uuid4(), storage_class="NumpyArray"))

def test_instantiate_via_butler_http_search(self):
"""Ensure that the primary Butler constructor's automatic search logic
correctly locates and reads the configuration file and ends up with a
RemoteButler pointing to the correct URL
"""

# This is kind of a fragile test. Butler's search logic does a lot of
# manipulations involving creating new ResourcePaths, and ResourcePath
# doesn't use httpx so we can't easily inject the TestClient in there.
Copy link
Member

@timj timj Nov 2, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we decide that we want to do that we can consider changing ResourcePath to use httpx rather than requests (Obviously this is not half a day's work but might not be a huge amount of work).

# We don't have an actual valid HTTP URL to give to the constructor
# because the test instance of the server is accessed via ASGI.
#
# Instead we just monkeypatch the HTTPResourcePath 'read' method and
# hope that all ResourcePath HTTP reads during construction are going
# to the server under test.
def override_read(http_resource_path):
return self.client.get(http_resource_path.geturl()).content

with patch.object(HttpResourcePath, "read", override_read):
butler = Butler("https://test.example/butler")
assert isinstance(butler, RemoteButler)
assert str(butler._config.remote_butler.url) == "https://test.example/butler/"


if __name__ == "__main__":
unittest.main()