crt: add option to run local #21

UweSchwaeke · 2026-01-22T11:28:46Z

crt: introduce --local-run option

what:
introduce an option flag signaling the crt tool to run locally. "locally" means the tool will not access remote repositories. commands and subcommands that explicitly require remote access will simply ignore the flag.
why:
this enables the crt tool to operate without a connection to remote repositories or the internet. it allows users to perform preparatory tasks—such as starting a release adding, removing manifests or adding patches, using only local data. a network connection is now only strictly required when finalizing the release.

jecluis

@UweSchwaeke I've left some comments on the various commits.

Additionally, we need a proper description on this Pull Request, including what you are attempting to do. I'm a bit lost on what you are actually trying to achieve (maybe I forgot some of our initial chat on this).

So it would be nice to have the context on the PR description so we know what we are aiming for.

jecluis · 2026-01-23T07:57:01Z

crt/src/crt/cmds/patchset.py

    if not ctx.local_run:
-        # ensure we have the specified branch in the ceph repo, so we can actually obtain
-        # the right shas
+        # ensure we have the specified branch in the ceph repo, so we can actually


This patch must be squashed/fixed up with the original commit introducing this change. We want a clean history of logical changes, not a history of all your changes regardless of what they are.

rewrote commits to squash some work in progress

jecluis · 2026-01-23T07:58:31Z

crt/src/crt/cmds/__init__.py

+def with_local_run(
+    f: Callable[Concatenate[bool, _P], _R],
+) -> Callable[_P, _R]:
+    """Pass the flag to don't access remotes from the context to the function."""
+
+    def inner(*args: _P.args, **kwargs: _P.kwargs) -> _R:
+        curr_ctx = click.get_current_context()
+        ctx = curr_ctx.find_object(Ctx)
+        if not ctx:
+            perror(f"missing context for '{f.__name__}'")
+            sys.exit(errno.ENOTRECOVERABLE)
+        return f(ctx.local_run, *args, **kwargs)
+
+    return update_wrapper(inner, f)
+
+


why are we adding this decorator? What benefit does that bring us?

this decorator is the same as with_patches_repo_path and with_gh_token. Some commands don't get the full Ctx. and therefore they get decorated with the with_,,, nethods to pass the option to it. At least this is myh understanding of those decorators

jecluis · 2026-01-23T08:00:28Z

crt/src/crt/cmds/release.py

-    src_repo: str,
-    dst_repo: str,
-    token: str,
+    ceph_repo_path: Path, src_repo: str, dst_repo: str, token: str, local_run: bool


please refrain from reformatting code that you do not need to reformat. This change should just be adding a new line to the end of the argument list, not rewriting the entirety of the argument list. You can let ruff do the right thing by adding a comma (,) at the end of local_run: bool.

thanks, i removed unnecessary reformatting

jecluis · 2026-01-23T08:01:09Z

crt/src/crt/cmds/release.py

+    if local_run:
+        return
+


We'll need to document why we're returning here. On a first pass it's not obvious to the reader.

i added a comment to the if statement

jecluis · 2026-01-23T08:01:53Z

crt/src/crt/cmds/release.py

-            sys.exit(errno.EEXIST)
+        if local_run:
+            if git_get_local_head(ceph_repo_path, dst_branch):
+                perror(f"destination branch '{dst_branch}' already exists local'")


s/local/locally/

also, why are we bailing out if the branch already exists locally? Is there any reason why we can't update the local branch from a remote branch?

i think because we shouldn't try to access remote branches if we run only local. i thought local_run is same as do not access remote repositories. Neither repositories located on github, nor repositories located on disk. Maybe i missed something.

i renamed the parameter to run_locally

jecluis · 2026-01-23T08:21:55Z

crt/src/crt/crtlib/manifest.py

    ceph_repo_path: Path,
    patches_repo_path: Path,
    token: str,
+    local_run: bool,


this should be a keyword argument defaulting to False.

thx, made all to keyword arguments with defaults to False

jecluis · 2026-01-23T08:24:51Z

crt/src/crt/crtlib/git_utils.py

+def git_remote(repo_path: Path, remote_name: str) -> git.Remote | None:
+    logger.info(f"get remote '{remote_name}'")
+
+    repo = git.Repo(repo_path)
+    try:
+        return repo.remote(remote_name)
+    except ValueError:
+        logger.debug(f"remote '{remote_name}' doesn't exists")
+
+    return None
+
+


Why do we need this change? It is not used in this patch at all.

thanks , i think i used it in another commit

jecluis · 2026-01-23T08:25:42Z

crt/src/crt/cmds/release.py

    git_get_remote_ref,
    git_prepare_remote,
    git_push,
+    git_remote,


The patch that introduces this function should be squashed/fixed up with this one. We should not have the same logical change scattered across different patches.

thx, i squashed and rewrote some commits

jecluis · 2026-01-23T08:27:40Z

crt/src/crt/cmds/release.py

+        if not (remote := git_remote(ceph_repo_path, dst_repo)):
+            pinfo(f"remote {dst_repo} doesn't exists local")
+            console.print(Padding(table, (1, 0, 1, 0)))
+            progress.done_task()
+            progress.stop()
+            return


I'm not seeing how remote here is used. Are we using git_remote() as the equivalent to a git_remote_exists() ?

no later on it will be used

jecluis · 2026-01-23T08:28:55Z

crt/src/crt/cmds/release.py

            manifest.dst_repo,
            release.release_repo,
            gh_token,
-            False,  # TODO pass local_run instead of hard coded


Why is this patch standalone if it's fixing something that we introduced earlier as a TODO? Are these changes different enough to be considered 2 logical changes? Or are they part of the same logical change?

i introduced this TODO. sorry for that

UweSchwaeke · 2026-01-28T07:33:56Z

@UweSchwaeke I've left some comments on the various commits.

Additionally, we need a proper description on this Pull Request, including what you are attempting to do. I'm a bit lost on what you are actually trying to achieve (maybe I forgot some of our initial chat on this).

So it would be nice to have the context on the PR description so we know what we are aiming for.

I added a description.

jecluis · 2026-01-29T10:17:36Z

@UweSchwaeke on the commits' subjects, I'll need you to add a colon (:) between the component being changed and the subject.

UweSchwaeke · 2026-01-29T10:19:17Z

@UweSchwaeke on the commits' subjects, I'll need you to add a colon (:) between the component being changed and the subject.

just a moment i will fix it

jecluis · 2026-01-29T10:20:48Z

Also, please check the commit messages for typos. One example, the first commit shows:

don't check if release allready exists in remote

s/allready/already/

Additionally, ensure the commit messages are properly capitalized where it makes sense. While we don't want subject lines to be capitalized (unless required, e.g., to describe a CONSTANT_NAME), the commit message itself should come as easy-to-read text. I would also suggest adding a blank line between the commit's subject and the commit message body.

avoid accessing remotes if the flag is set and the subcommand doesn't require remote access. affected subcommands: * start: * skip adding remote URLs. * skip checking if the release already exists on the remote. * create a local release branch from base_ref (which must exist locally). * do not push the created branch or tag to the remote. * list: * skip adding remote urls. * skip fetching from remotes. other subcommands will ignore the flag. Signed-off-by: Uwe Schwaeke <uwe.schwaeke@clyso.com>

don't access remotes if the flag is set and the subcommand doesn't require remote access. affected subcommands: * patchset add: * don't add remote urls and don't fetch patch branch (assume branch exists locally) * validate: * don't add remote urls and don't fetch from remote other subcommands will ignore the flag Signed-off-by: Uwe Schwaeke <uwe.schwaeke@clyso.com>

don't access remotes if the flag is set and the subcommand doesn't require remote access. affected subcommands: * add: * don't add remote url and don't fetch Signed-off-by: Uwe Schwaeke <uwe.schwaeke@clyso.com>

don't access remotes if the flag is set and the subcommand doesn't require remote access. affected subcommands: * add: * don't add remote url and don't fetch * create branch from an existing local branch * publish: * don't add remote url and don't fetch * create branch from an existing local branch other subcommands will ignore the flag Signed-off-by: Uwe Schwaeke <uwe.schwaeke@clyso.com>

UweSchwaeke · 2026-01-29T14:08:59Z

Also, please check the commit messages for typos. One example, the first commit shows:

don't check if release allready exists in remote

s/allready/already/

Additionally, ensure the commit messages are properly capitalized where it makes sense. While we don't want subject lines to be capitalized (unless required, e.g., to describe a CONSTANT_NAME), the commit message itself should come as easy-to-read text. I would also suggest adding a blank line between the commit's subject and the commit message body.

i added a better description ( mostly reviewed with gemini)

UweSchwaeke requested a review from jecluis January 22, 2026 12:55

jecluis requested changes Jan 23, 2026

View reviewed changes

UweSchwaeke force-pushed the wip-crt-local-run branch from d7afa8f to 6c0c518 Compare January 27, 2026 14:18

UweSchwaeke added 4 commits January 29, 2026 11:24

crt/cmds/patch: add --local-run option

ef439d9

don't access remotes if the flag is set and the subcommand doesn't require remote access. affected subcommands: * add: * don't add remote url and don't fetch Signed-off-by: Uwe Schwaeke <uwe.schwaeke@clyso.com>

UweSchwaeke force-pushed the wip-crt-local-run branch from 6c0c518 to 6ec1cfe Compare January 29, 2026 13:55

UweSchwaeke marked this pull request as ready for review January 30, 2026 05:43

crt: add option to run local #21

Are you sure you want to change the base?

crt: add option to run local #21

Uh oh!

Conversation

UweSchwaeke commented Jan 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jecluis left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

UweSchwaeke commented Jan 28, 2026

Uh oh!

jecluis commented Jan 29, 2026

Uh oh!

UweSchwaeke commented Jan 29, 2026

Uh oh!

jecluis commented Jan 29, 2026

Uh oh!

UweSchwaeke commented Jan 29, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

UweSchwaeke commented Jan 22, 2026 •

edited

Loading