-
Notifications
You must be signed in to change notification settings - Fork 2
kpd: add optional mirror support #16
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
kpd: add optional mirror support #16
Conversation
We always clone a full repository. This is counter productive and wasteful. Allow users to specify that they are using kpd to help test patches for a git tree which we should expect a mirror on a target mirror path. Optionally, we also allow users to clarify that their target git tree is a linux clone, and in such cases we can always fallback to looking for a mirror path with the "linux.git" name. So for example, all kdevops enterprise deployments can easily profit from this as kdevops has support to mirror all target git trees it supports under /mirror/ through an NFS export for clients. And so small thing guests can be used for kpd instances, which can leverage this NFS export. This allows kpd to be run on smaller guests with less storage needs. This should allow more than one kpd instance to run on small guests too. Generated-by: ChatGPT Codex Signed-off-by: Luis Chamberlain <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A great improvement overall, thanks!
Please see comments with feedback.
mirror_dir=kpd_config.mirror_dir, | ||
linux_clone=kpd_config.linux_clone, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since new config options are parameters of BranchWorker
, let's make them branch-worker specific in the configuration file too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure.
"base_directory": "/tmp/repos" | ||
"base_directory": "/tmp/repos", | ||
"mirror_dir": "/mirror/", | ||
"linux_clone": true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"linux_clone"
is a confusing name for this flag, and also hardcoding "linux.git"
seems unnecessary.
How about we change this option to smth like "mirror_fallback_repo"
, and set it to the path or git url of the fallback repository?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure..
the git clone --reference option when cloning. Using this can save considerable bandwidth and | ||
space, allowing kpd to run on thing guests on a corporate environment with for example an NFS | ||
mount for local git trees on a network. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this section belongs to the README file.
It would be great to have a separate document about the config, if you don't mind making a draft of that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure
): | ||
reference_path = fallback | ||
if os.path.exists(reference_path): | ||
multi_opts = ["--reference", reference_path] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TIL git clone --reference
, very cool.
nit: Is it more appropriate to use --reference-if-able
(man git-clone)? Although we already checked the path in python, so it probably doesn't matter.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was not aware of --reference-if-able. yes. Good.
if multi_opts: | ||
repo = git.Repo.clone_from(url, path, multi_options=multi_opts) | ||
else: | ||
repo = git.Repo.clone_from(url, path) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it makes a lot of sense to always have mirror_dir
and use the option only to specify a path. And then effectively use it as a cache.
If a mirror doesn't exist, clone it from scratch to mirror_dir
. And then always use --reference
when syncing.
What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes perfect sense to me. I'll see if codex can parse, "look a this URL and make all the suggested changes".
We always clone a full repository. This is counter productive and wasteful. Allow users to specify that they are using kpd to help test patches for a git tree which we should expect a mirror on a target mirror path.
Optionally, we also allow users to clarify that their target git tree is a linux clone, and in such cases we can always fallback to looking for a mirror path with the "linux.git" name.
So for example, all kdevops enterprise deployments can easily profit from this as kdevops has support to mirror all target git trees it supports under /mirror/ through an NFS export for clients. And so small thing guests can be used for kpd instances, which can leverage this NFS export.
This allows kpd to be run on smaller guests with less storage needs. This should allow more than one kpd instance to run on small guests too.
Generated-by: ChatGPT Codex