From d89f51f6760ab66e81a64ab33ea07b297f37088c Mon Sep 17 00:00:00 2001 From: natthan-pigoux Date: Tue, 14 Jan 2025 17:41:49 +0100 Subject: [PATCH 1/3] fix: parse queries in url to get branch and ensure checkout --- diracx-core/src/diracx/core/config/sources.py | 21 ++++++++++++++++--- diracx-core/tests/test_config_source.py | 8 ++++--- 2 files changed, 23 insertions(+), 6 deletions(-) diff --git a/diracx-core/src/diracx/core/config/sources.py b/diracx-core/src/diracx/core/config/sources.py index 2dd6eaef..f14e568f 100644 --- a/diracx-core/src/diracx/core/config/sources.py +++ b/diracx-core/src/diracx/core/config/sources.py @@ -13,6 +13,7 @@ from pathlib import Path from tempfile import TemporaryDirectory from typing import Annotated +from urllib.parse import parse_qs, urlparse, urlunparse import sh import yaml @@ -136,13 +137,15 @@ def __init__(self, *, backend_url: ConfigSourceUrl) -> None: MAX_CS_CACHED_VERSIONS, DEFAULT_CS_CACHE_TTL ) self._read_raw_cache: Cache = LRUCache(MAX_CS_CACHED_VERSIONS) + self.remote_url = self.exctract_remote_url(backend_url) + self.git_branch = self.get_git_branch_from_url(backend_url) @cachedmethod(lambda self: self._latest_revision_cache) def latest_revision(self) -> tuple[str, datetime]: try: rev = sh.git( "rev-parse", - DEFAULT_GIT_BRANCH, + self.git_branch, _cwd=self.repo_location, _tty_out=False, _async=is_running_in_async_context(), @@ -192,6 +195,18 @@ def clear_caches(self): self._latest_revision_cache.clear() self._read_raw_cache.clear() + def exctract_remote_url(self, backend_url: ConfigSourceUrl) -> str: + """Extract the base URL without the 'git+' prefix and query parameters.""" + parsed_url = urlparse(str(backend_url).replace("git+", "")) + remote_url = urlunparse(parsed_url._replace(query="")) + return remote_url + + def get_git_branch_from_url(self, backend_url: ConfigSourceUrl) -> str: + """Extract the branch from the query parameters.""" + parsed_url = urlparse(str(backend_url)) + branch = parse_qs(parsed_url.query).get("branch", [DEFAULT_GIT_BRANCH])[0] + return branch + class LocalGitConfigSource(BaseGitConfigSource): """The configuration is stored on a local git repository @@ -219,6 +234,7 @@ def __init__(self, *, backend_url: ConfigSourceUrl) -> None: raise ValueError( f"{self.repo_location} is not a valid git repository" ) from e + sh.git.checkout(self.git_branch, _cwd=self.repo_location, _async=False) def __hash__(self): return hash(self.repo_location) @@ -234,14 +250,13 @@ def __init__(self, *, backend_url: ConfigSourceUrl) -> None: if not backend_url: raise ValueError("No remote url for RemoteGitConfigSource") - # git does not understand `git+https`, so we remove the `git+` part - self.remote_url = str(backend_url).replace("git+", "") self._temp_dir = TemporaryDirectory() self.repo_location = Path(self._temp_dir.name) sh.git.clone(self.remote_url, self.repo_location, _async=False) self._pull_cache: Cache = TTLCache( MAX_PULL_CACHED_VERSIONS, DEFAULT_PULL_CACHE_TTL ) + sh.git.checkout(self.git_branch, _cwd=self.repo_location, _async=False) def clear_caches(self): super().clear_caches() diff --git a/diracx-core/tests/test_config_source.py b/diracx-core/tests/test_config_source.py index a889690f..9be7576b 100644 --- a/diracx-core/tests/test_config_source.py +++ b/diracx-core/tests/test_config_source.py @@ -9,7 +9,8 @@ from diracx.core.config.schema import Config # The diracx-chart contains a CS example -TEST_REPO = "git+https://github.com/DIRACGrid/diracx-charts/" +TEST_REPO = "git+https://github.com/DIRACGrid/diracx-charts.git" +TEST_REPO_SPECIFIC_BRANCH = TEST_REPO + "?branch=master" def github_is_down(): @@ -22,13 +23,14 @@ def github_is_down(): @pytest.mark.skipif(github_is_down(), reason="Github unavailble") -def test_remote_git_config_source(monkeypatch): +@pytest.mark.parametrize("repo_url", [TEST_REPO, TEST_REPO_SPECIFIC_BRANCH]) +def test_remote_git_config_source(monkeypatch, repo_url): monkeypatch.setattr( "diracx.core.config.sources.DEFAULT_CONFIG_FILE", "k3s/examples/cs.yaml", ) - remote_conf = ConfigSource.create_from_url(backend_url=TEST_REPO) + remote_conf = ConfigSource.create_from_url(backend_url=repo_url) assert isinstance(remote_conf, RemoteGitConfigSource) hexsha, modified = remote_conf.latest_revision() From eeadb108ac415c0d612e7d51c3d675d5c09df578 Mon Sep 17 00:00:00 2001 From: natthan-pigoux Date: Wed, 15 Jan 2025 09:17:57 +0100 Subject: [PATCH 2/3] use ConfigSource instead of urlparse --- diracx-core/src/diracx/core/config/sources.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/diracx-core/src/diracx/core/config/sources.py b/diracx-core/src/diracx/core/config/sources.py index f14e568f..f1341edc 100644 --- a/diracx-core/src/diracx/core/config/sources.py +++ b/diracx-core/src/diracx/core/config/sources.py @@ -13,7 +13,7 @@ from pathlib import Path from tempfile import TemporaryDirectory from typing import Annotated -from urllib.parse import parse_qs, urlparse, urlunparse +from urllib.parse import urlparse, urlunparse import sh import yaml @@ -197,15 +197,14 @@ def clear_caches(self): def exctract_remote_url(self, backend_url: ConfigSourceUrl) -> str: """Extract the base URL without the 'git+' prefix and query parameters.""" + # TODO: fix typing issue parsed_url = urlparse(str(backend_url).replace("git+", "")) remote_url = urlunparse(parsed_url._replace(query="")) return remote_url def get_git_branch_from_url(self, backend_url: ConfigSourceUrl) -> str: """Extract the branch from the query parameters.""" - parsed_url = urlparse(str(backend_url)) - branch = parse_qs(parsed_url.query).get("branch", [DEFAULT_GIT_BRANCH])[0] - return branch + return dict(backend_url.query_params()).get("branch", DEFAULT_GIT_BRANCH) class LocalGitConfigSource(BaseGitConfigSource): From e11d4172c300c53d68fa5e94536dcc07f1f7dd53 Mon Sep 17 00:00:00 2001 From: natthan-pigoux Date: Wed, 15 Jan 2025 09:22:28 +0100 Subject: [PATCH 3/3] remove comment --- diracx-core/src/diracx/core/config/sources.py | 1 - 1 file changed, 1 deletion(-) diff --git a/diracx-core/src/diracx/core/config/sources.py b/diracx-core/src/diracx/core/config/sources.py index f1341edc..217e99c1 100644 --- a/diracx-core/src/diracx/core/config/sources.py +++ b/diracx-core/src/diracx/core/config/sources.py @@ -197,7 +197,6 @@ def clear_caches(self): def exctract_remote_url(self, backend_url: ConfigSourceUrl) -> str: """Extract the base URL without the 'git+' prefix and query parameters.""" - # TODO: fix typing issue parsed_url = urlparse(str(backend_url).replace("git+", "")) remote_url = urlunparse(parsed_url._replace(query="")) return remote_url