[libvirt PATCH v3 0/6] ci: helper: Rewrite image listing in Python and add stale Docker image checker
since v2: - dropped original patch 1 renaming ci-help to help in Makefile - changed the --quiet cmdline option to be tied to the refresh cmd only - urrlib imports fixed - improved on Python type hinting - improved util functions docstrings - replaced all tabs in formatted strings with 4 spaces - moved the list_stale_images method to util as requested - renamed variables in list_stale_images so that it's more clear at first glance what they actually hold - added a hint on where GitLab personal tokens should be generated - added a patch converting private method naming to reflect the common Python practice (I did it in a separate patch so that if this is not accepted I only need to ditch a single patch with no further changes) v1: https://listman.redhat.com/archives/libvir-list/2021-February/msg00641.ht= ml v2: https://listman.redhat.com/archives/libvir-list/2021-March/msg00789.html EXAMPLE OUTPUT OF THE REGISTRY CHECKER: The following images can be purged from the registry: ci-debian-9: (ID: 1154661) ci-debian-9-cross-aarch64: (ID: 1154667) ci-debian-9-cross-mipsel: (ID: 1154669) ci-debian-9-cross-armv7l: (ID: 1154671) ci-debian-9-cross-armv6l: (ID: 1154676) ci-debian-9-cross-mips: (ID: 1154678) ci-debian-9-cross-ppc64le: (ID: 1154682) ci-debian-9-cross-s390x: (ID: 1154683) ci-debian-9-cross-mips64el: (ID: 1154686) ci-fedora-31: (ID: 1154687) ci-opensuse-151: (ID: 1154724) You can remove the above images over the API with the following code snippet: $ for image_id in 1154661 1154667 1154669 1154671 1154676 1154678 1154682= \ 1154683 1154686 1154687 1154724; do \ curl --request DELETE --header "PRIVATE-TOKEN: <access_token>" \ https://gitlab.com/api/v4/projects/libvirt%2Flibvirt/registry/repos= itories/$image_id \ done You can generate a personal access token here: https://gitlab.com/-/profile/personal_access_tokens Erik Skultety (6): ci: helper: Use the ArgumentDefaultsHelpFormatter help formatter ci: helper: Introduce --quiet ci: Introduce a util module ci: helper: Rewrite image listing to Python ci: util: Add a registry checker for stale images ci: helper: Convert private methods to Python common naming practice ci/Makefile | 12 ---- ci/helper | 162 ++++++++++++++++++++++++++++++++++++---------- ci/list-images.sh | 14 ---- ci/util.py | 92 ++++++++++++++++++++++++++ 4 files changed, 219 insertions(+), 61 deletions(-) delete mode 100644 ci/list-images.sh create mode 100644 ci/util.py --=20 2.29.2
This help formatter class reports the defaults we use for options taking an argument. Signed-off-by: Erik Skultety <eskultet@redhat.com> Reviewed-by: Andrea Bolognani <abologna@redhat.com> --- ci/helper | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/ci/helper b/ci/helper index 73a3f729da..8f34f4b59d 100755 --- a/ci/helper +++ b/ci/helper @@ -79,6 +79,7 @@ class Parser: "build", help="run a build in a container", parents=[containerparser, mesonparser], + formatter_class=argparse.ArgumentDefaultsHelpFormatter, ) buildparser.set_defaults(func=Application.action_build) @@ -87,6 +88,7 @@ class Parser: "test", help="run a build in a container (including tests)", parents=[containerparser, mesonparser], + formatter_class=argparse.ArgumentDefaultsHelpFormatter, ) testparser.set_defaults(func=Application.action_test) @@ -95,6 +97,7 @@ class Parser: "shell", help="start a shell in a container", parents=[containerparser], + formatter_class=argparse.ArgumentDefaultsHelpFormatter, ) shellparser.set_defaults(func=Application.action_shell) @@ -102,6 +105,7 @@ class Parser: listimagesparser = subparsers.add_parser( "list-images", help="list known container images", + formatter_class=argparse.ArgumentDefaultsHelpFormatter, ) listimagesparser.set_defaults(func=Application.action_list_images) @@ -110,6 +114,7 @@ class Parser: "refresh", help="refresh data generated with lcitool", parents=[lcitoolparser], + formatter_class=argparse.ArgumentDefaultsHelpFormatter, ) refreshparser.set_defaults(func=Application.action_refresh) -- 2.29.2
Offer an option to silence all output to stdout, e.g. when generating dockerfiles/varsfiles. Signed-off-by: Erik Skultety <eskultet@redhat.com> Reviewed-by: Andrea Bolognani <abologna@redhat.com> --- ci/helper | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/ci/helper b/ci/helper index 8f34f4b59d..53baef3cb1 100755 --- a/ci/helper +++ b/ci/helper @@ -116,6 +116,12 @@ class Parser: parents=[lcitoolparser], formatter_class=argparse.ArgumentDefaultsHelpFormatter, ) + refreshparser.add_argument( + "--quiet", + action="store_true", + default=False, + help="refresh data silently" + ) refreshparser.set_defaults(func=Application.action_refresh) def parse(self): @@ -173,7 +179,8 @@ class Application: outfile = f"ci-{host}-cross-{cross}.Dockerfile" outpath = outdir.joinpath(outfile) - print(outpath) + if not self.args.quiet: + print(outpath) output = self.lcitool_run(args) with open(outpath, "w") as f: @@ -185,7 +192,8 @@ class Application: outfile = f"{host}.vars" outpath = outdir.joinpath(outfile) - print(outpath) + if not self.args.quiet: + print(outpath) output = self.lcitool_run(args) with open(outpath, "w") as f: -- 2.29.2
With the gradual rewrite of the Makefile to the 'helper' script will require helper functions that would better live in a separate util module. Signed-off-by: Erik Skultety <eskultet@redhat.com> Reviewed-by: Andrea Bolognani <abologna@redhat.com> --- ci/util.py | 40 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) create mode 100644 ci/util.py diff --git a/ci/util.py b/ci/util.py new file mode 100644 index 0000000000..f9f8320276 --- /dev/null +++ b/ci/util.py @@ -0,0 +1,40 @@ +import json +import urllib.request +import urllib.parse + +from typing import Dict, List + + +def get_registry_uri(namespace: str, + gitlab_uri: str = "https://gitlab.com") -> str: + """ + Construct a v4 API URI pointing the namespaced project's image registry. + + :param namespace: GitLab project namespace, e.g. "libvirt/libvirt" + :param gitlab_uri: GitLab base URI, can be a private deployment + :param api_version: GitLab REST API version number + :return: URI pointing to a namespaced project's image registry + """ + + # this converts something like "libvirt/libvirt" to "libvirt%2Flibvirt" + namespace_urlenc = urllib.parse.quote_plus(namespace) + + project_uri = f"{gitlab_uri}/api/v4/projects/{namespace_urlenc}" + + uri = project_uri + "/registry/repositories" + return uri + + +def get_registry_images(uri: str) -> List[Dict]: + """ + List all container images that are currently available in the given GitLab + project. + + :param uri: URI pointing to a GitLab instance's image registry + :return: list of container image names + """ + + r = urllib.request.urlopen(uri + "?per_page=100") + + # read the HTTP response and load the JSON part of it + return json.loads(r.read().decode()) -- 2.29.2
The corresponding Bash script is dropped. After this patch's rewrite, the Makefile's original image listing target remains intact only to notify the user to use the Python helper instead. Signed-off-by: Erik Skultety <eskultet@redhat.com> Reviewed-by: Andrea Bolognani <abologna@redhat.com> --- ci/Makefile | 12 ------------ ci/helper | 38 +++++++++++++++++++++++++++++++++++++- ci/list-images.sh | 14 -------------- 3 files changed, 37 insertions(+), 27 deletions(-) delete mode 100644 ci/list-images.sh diff --git a/ci/Makefile b/ci/Makefile index 72f5bda942..02ce0df7ee 100644 --- a/ci/Makefile +++ b/ci/Makefile @@ -211,17 +211,6 @@ ci-build@%: ci-test@%: $(MAKE) -C $(CI_ROOTDIR) ci-build@$* CI_NINJA_ARGS=test -ci-list-images: - @echo - @echo "Available x86 container images:" - @echo - @sh list-images.sh "$(CI_IMAGE_PREFIX)" | grep -v cross - @echo - @echo "Available cross-compiler container images:" - @echo - @sh list-images.sh "$(CI_IMAGE_PREFIX)" | grep cross - @echo - ci-help: @echo @echo @@ -241,7 +230,6 @@ ci-help: @echo " ci-build@\$$IMAGE - run a default 'ninja' build" @echo " ci-test@\$$IMAGE - run a 'ninja test'" @echo " ci-shell@\$$IMAGE - run an interactive shell" - @echo " ci-list-images - list available images" @echo " ci-help - show this help message" @echo @echo "Available make variables:" diff --git a/ci/helper b/ci/helper index 53baef3cb1..31cf72fbdf 100755 --- a/ci/helper +++ b/ci/helper @@ -11,6 +11,8 @@ import shutil import subprocess import sys +import util + class Parser: def __init__(self): @@ -66,6 +68,20 @@ class Parser: help="path to lcitool binary", ) + # Options that are common to actions communicating with a GitLab + # instance + gitlabparser = argparse.ArgumentParser(add_help=False) + gitlabparser.add_argument( + "--namespace", + default="libvirt/libvirt", + help="GitLab project namespace" + ) + gitlabparser.add_argument( + "--gitlab-uri", + default="https://gitlab.com", + help="base GitLab URI" + ) + # Main parser self.parser = argparse.ArgumentParser() subparsers = self.parser.add_subparsers( @@ -105,6 +121,7 @@ class Parser: listimagesparser = subparsers.add_parser( "list-images", help="list known container images", + parents=[gitlabparser], formatter_class=argparse.ArgumentDefaultsHelpFormatter, ) listimagesparser.set_defaults(func=Application.action_list_images) @@ -249,7 +266,26 @@ class Application: self.make_run(f"ci-shell@{self.args.target}") def action_list_images(self): - self.make_run(f"ci-list-images") + registry_uri = util.get_registry_uri(self.args.namespace, + self.args.gitlab_uri) + images = util.get_registry_images(registry_uri) + + # skip the "ci-" prefix each of our container images' name has + name_prefix = "ci-" + names = [i["name"][len(name_prefix):] for i in images] + names.sort() + + native = [name for name in names if "-cross-" not in name] + cross = [name for name in names if "-cross-" in name] + + spacing = 4 * " " + print("Available x86 container images:\n") + print(spacing + ("\n" + spacing).join(native)) + + if cross: + print() + print("Available cross-compiler container images:\n") + print(spacing + ("\n" + spacing).join(cross)) def action_refresh(self): self.refresh_containers() diff --git a/ci/list-images.sh b/ci/list-images.sh deleted file mode 100644 index b85b132253..0000000000 --- a/ci/list-images.sh +++ /dev/null @@ -1,14 +0,0 @@ -#!/bin/sh - -prefix="${1##registry.gitlab.com/}" - -PROJECT_ID=192693 - -all_repos() { - curl -s "https://gitlab.com/api/v4/projects/$PROJECT_ID/registry/repositories?per_pag..." \ - | tr , '\n' | grep '"path":' | sed 's,"path":",,g;s,"$,,g' -} - -all_repos | grep "^$prefix" | sed "s,^$prefix,,g" | while read repo; do - echo " $repo" -done | sort -u -- 2.29.2
This function checks whether there are any stale Docker images in the registry that can be purged. Since we're pulling available container images from our GitLab registry with the 'list-images' action, it could happen that we'd list old (already unsupported) images and make them available for the user to consume and run a build in them. Naturally, the build will most likely fail leaving the user confused. Signed-off-by: Erik Skultety <eskultet@redhat.com> --- ci/helper | 45 ++++++++++++++++++++++++++++++++++++++++++++- ci/util.py | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 96 insertions(+), 1 deletion(-) diff --git a/ci/helper b/ci/helper index 31cf72fbdf..b5255db835 100755 --- a/ci/helper +++ b/ci/helper @@ -130,7 +130,7 @@ class Parser: refreshparser = subparsers.add_parser( "refresh", help="refresh data generated with lcitool", - parents=[lcitoolparser], + parents=[lcitoolparser, gitlabparser], formatter_class=argparse.ArgumentDefaultsHelpFormatter, ) refreshparser.add_argument( @@ -139,6 +139,13 @@ class Parser: default=False, help="refresh data silently" ) + refreshparser.add_argument( + "--check-stale", + action="store", + choices=["yes", "no"], + default="yes", + help="check for existence of stale images on the GitLab instance" + ) refreshparser.set_defaults(func=Application.action_refresh) def parse(self): @@ -287,10 +294,46 @@ class Application: print("Available cross-compiler container images:\n") print(spacing + ("\n" + spacing).join(cross)) + def check_stale_images(self): + if self.args.check_stale != "yes" or self.args.quiet: + return + + namespace = self.args.namespace + gitlab_uri = self.args.gitlab_uri + registry_uri = util.get_registry_uri(namespace, gitlab_uri) + lcitool_hosts = self.lcitool_get_hosts() + + stale_images = util.get_registry_stale_images(registry_uri, + lcitool_hosts) + if stale_images: + spacing = "\n" + 4 * " " + stale_fmt = [f"{k} (ID: {v})" for k, v in stale_images.items()] + stale_details = spacing + spacing.join(stale_fmt) + stale_ids = ' '.join([str(id) for id in stale_images.values()]) + registry_uri = util.get_registry_uri(namespace, gitlab_uri) + + print(f""" +The following images are stale and can be purged from the registry: +{stale_details} + +You can remove the above images over the API with the following code snippet: + + $ for image_id in {stale_ids}; do \\ + curl --request DELETE --header "PRIVATE-TOKEN: <access_token>" \\ + {registry_uri}/$image_id \\ + done + +You can generate a personal access token here: + {gitlab_uri}/-/profile/personal_access_tokens""") + def action_refresh(self): + # refresh Dockerfiles and vars files self.refresh_containers() self.refresh_cirrus() + # check for stale images + self.check_stale_images() + def run(self): self.args.func(self) diff --git a/ci/util.py b/ci/util.py index f9f8320276..d69c246872 100644 --- a/ci/util.py +++ b/ci/util.py @@ -38,3 +38,55 @@ def get_registry_images(uri: str) -> List[Dict]: # read the HTTP response and load the JSON part of it return json.loads(r.read().decode()) + + +def get_image_distro(image_name: str) -> str: + """ + Extract the name of the distro in the GitLab image registry name, e.g. + ci-debian-9-cross-mipsel --> debian-9 + + :param image_name: name of the GitLab registry image + :return: distro name as a string + """ + name_prefix = "ci-" + name_suffix = "-cross-" + + distro = image_name[len(name_prefix):] + + index = distro.find(name_suffix) + if index > 0: + distro = distro[:index] + + return distro + + +def get_registry_stale_images(registry_uri: str, + supported_distros: List[str]) -> Dict[str, int]: + """ + Check the GitLab image registry for images that we no longer support and + which should be deleted. + + :param uri: URI pointing to a GitLab instance's image registry + :param supported_distros: list of hosts supported by lcitool + :return: dictionary formatted as: {<gitlab_image_name>: <gitlab_image_id>} + """ + + images = get_registry_images(registry_uri) + + # extract distro names from the list of registry images + registry_distros = [get_image_distro(i["name"]) for i in images] + + # - compare the distros powering the images in GitLab registry with + # the list of host available from lcitool + # - @unsupported is a set containing the distro names which we no longer + # support; we need to map these back to registry image names + unsupported = set(registry_distros) - set(supported_distros) + if unsupported: + stale_images = {} + for distro in unsupported: + for img in images: + # gitlab images are named like "ci-<distro>-<cross_arch>?" + if distro in img["name"]: + stale_images[img["name"]] = img["id"] + + return stale_images -- 2.29.2
On Thu, 2021-03-18 at 09:09 +0100, Erik Skultety wrote:
+ def check_stale_images(self): + if self.args.check_stale != "yes" or self.args.quiet: + return
I would prefer it if this check were to be performed in the action_refresh() method to decide whether or not check_stale_images() should be called.
+ print(f""" +The following images are stale and can be purged from the registry: +{stale_details} + +You can remove the above images over the API with the following code snippet:
"You can delete the images listed above using this shell snippet:"
+ $ for image_id in {stale_ids}; do \\ + curl --request DELETE --header "PRIVATE-TOKEN: <access_token>" \\ + {registry_uri}/$image_id \\ + done
This will not result in a working shell snippet being displayed. What you want is f""" $ for image_id in {stale_ids}; do curl --request DELETE --header "PRIVATE-TOKEN: <access_token>" \\ {registry_uri}/$image_id; done """
+ +You can generate a personal access token here: + {gitlab_uri}/-/profile/personal_access_tokens""")
Empty line before the URL for readability. I still think that we should use textwrap.dedent(), as it makes the script much more readable at the cost of a single additional string.replace() call: if stale_images: spacing = "\n" + 4 * " " stale_fmt = [f"{k} (ID: {v})" for k, v in stale_images.items()] stale_details = spacing.join(stale_fmt) stale_ids = ' '.join([str(id) for id in stale_images.values()]) registry_uri = util.get_registry_uri(namespace, gitlab_uri) msg = textwrap.dedent(f""" The following images are stale and can be purged from the registry: STALE_DETAILS You can delete the images listed above using this shell snippet: $ for image_id in {stale_ids}; do curl --request DELETE --header "PRIVATE-TOKEN: <access_token>" \\ {registry_uri}/$image_id; \\ done You can generate a personal access token here: {gitlab_uri}/-/profile/personal_access_tokens """) print(msg.replace("STALE_DETAILS", stale_details))
def action_refresh(self): + # refresh Dockerfiles and vars files self.refresh_containers() self.refresh_cirrus()
+ # check for stale images + self.check_stale_images()
The method names are really quite self-explanatory, so the comments you're introducing don't add much IMHO... I'd say leave them out.
+def get_registry_stale_images(registry_uri: str, + supported_distros: List[str]) -> Dict[str, int]: + """ + Check the GitLab image registry for images that we no longer support and + which should be deleted. + + :param uri: URI pointing to a GitLab instance's image registry + :param supported_distros: list of hosts supported by lcitool + :return: dictionary formatted as: {<gitlab_image_name>: <gitlab_image_id>} + """ + + images = get_registry_images(registry_uri) + + # extract distro names from the list of registry images + registry_distros = [get_image_distro(i["name"]) for i in images] + + # - compare the distros powering the images in GitLab registry with + # the list of host available from lcitool + # - @unsupported is a set containing the distro names which we no longer + # support; we need to map these back to registry image names + unsupported = set(registry_distros) - set(supported_distros) + if unsupported: + stale_images = {} + for distro in unsupported: + for img in images: + # gitlab images are named like "ci-<distro>-<cross_arch>?" + if distro in img["name"]: + stale_images[img["name"]] = img["id"] + + return stale_images
As far as I can tell, this can be achieved in a much more straightforward way with def get_registry_stale_images(registry_uri, supported_distros): images = get_registry_images(registry_uri) stale_images = {} for img in images: if get_image_distro(img["name"]) not in supported_distros: stale_images[img["name"]] = img["id"] return stale_images At least from a quick test, the results appear to be the same. Am I missing something? -- Andrea Bolognani / Red Hat / Virtualization
...
+def get_registry_stale_images(registry_uri: str, + supported_distros: List[str]) -> Dict[str, int]: + """ + Check the GitLab image registry for images that we no longer support and + which should be deleted. + + :param uri: URI pointing to a GitLab instance's image registry + :param supported_distros: list of hosts supported by lcitool + :return: dictionary formatted as: {<gitlab_image_name>: <gitlab_image_id>} + """ + + images = get_registry_images(registry_uri) + + # extract distro names from the list of registry images + registry_distros = [get_image_distro(i["name"]) for i in images] + + # - compare the distros powering the images in GitLab registry with + # the list of host available from lcitool + # - @unsupported is a set containing the distro names which we no longer + # support; we need to map these back to registry image names + unsupported = set(registry_distros) - set(supported_distros) + if unsupported: + stale_images = {} + for distro in unsupported: + for img in images: + # gitlab images are named like "ci-<distro>-<cross_arch>?" + if distro in img["name"]: + stale_images[img["name"]] = img["id"] + + return stale_images
As far as I can tell, this can be achieved in a much more straightforward way with
def get_registry_stale_images(registry_uri, supported_distros):
images = get_registry_images(registry_uri) stale_images = {}
for img in images: if get_image_distro(img["name"]) not in supported_distros: stale_images[img["name"]] = img["id"]
return stale_images
At least from a quick test, the results appear to be the same. Am I missing something?
No, it works. I'll respin. Erik
As documented at [1], the common practice wrt to private attributes/methods naming is to prefix them with an underscore. [1] https://docs.python.org/3/tutorial/classes.html#private-variables Signed-off-by: Erik Skultety <eskultet@redhat.com> --- ci/helper | 68 +++++++++++++++++++++++++++---------------------------- 1 file changed, 34 insertions(+), 34 deletions(-) diff --git a/ci/helper b/ci/helper index b5255db835..3d2e0397f5 100755 --- a/ci/helper +++ b/ci/helper @@ -97,7 +97,7 @@ class Parser: parents=[containerparser, mesonparser], formatter_class=argparse.ArgumentDefaultsHelpFormatter, ) - buildparser.set_defaults(func=Application.action_build) + buildparser.set_defaults(func=Application._action_build) # test action testparser = subparsers.add_parser( @@ -106,7 +106,7 @@ class Parser: parents=[containerparser, mesonparser], formatter_class=argparse.ArgumentDefaultsHelpFormatter, ) - testparser.set_defaults(func=Application.action_test) + testparser.set_defaults(func=Application._action_test) # shell action shellparser = subparsers.add_parser( @@ -115,7 +115,7 @@ class Parser: parents=[containerparser], formatter_class=argparse.ArgumentDefaultsHelpFormatter, ) - shellparser.set_defaults(func=Application.action_shell) + shellparser.set_defaults(func=Application._action_shell) # list-images action listimagesparser = subparsers.add_parser( @@ -124,7 +124,7 @@ class Parser: parents=[gitlabparser], formatter_class=argparse.ArgumentDefaultsHelpFormatter, ) - listimagesparser.set_defaults(func=Application.action_list_images) + listimagesparser.set_defaults(func=Application._action_list_images) # refresh action refreshparser = subparsers.add_parser( @@ -146,7 +146,7 @@ class Parser: default="yes", help="check for existence of stale images on the GitLab instance" ) - refreshparser.set_defaults(func=Application.action_refresh) + refreshparser.set_defaults(func=Application._action_refresh) def parse(self): return self.parser.parse_args() @@ -161,7 +161,7 @@ class Application: if not shutil.which(self.args.lcitool): sys.exit("error: 'lcitool' not installed") - def make_run(self, target): + def _make_run(self, target): args = [ "-C", self.basedir, @@ -185,15 +185,15 @@ class Application: if pty.spawn(["make"] + args) != 0: sys.exit("error: 'make' failed") - def lcitool_run(self, args): + def _lcitool_run(self, args): output = subprocess.check_output([self.args.lcitool] + args) return output.decode("utf-8") - def lcitool_get_hosts(self): - output = self.lcitool_run(["hosts"]) + def _lcitool_get_hosts(self): + output = self._lcitool_run(["hosts"]) return output.splitlines() - def generate_dockerfile(self, host, cross=None): + def _generate_dockerfile(self, host, cross=None): args = ["dockerfile", host, "libvirt"] outdir = self.basedir.joinpath("containers") outfile = f"ci-{host}.Dockerfile" @@ -206,11 +206,11 @@ class Application: if not self.args.quiet: print(outpath) - output = self.lcitool_run(args) + output = self._lcitool_run(args) with open(outpath, "w") as f: f.write(output) - def generate_vars(self, host): + def _generate_vars(self, host): args = ["variables", host, "libvirt"] outdir = self.basedir.joinpath("cirrus") outfile = f"{host}.vars" @@ -219,11 +219,11 @@ class Application: if not self.args.quiet: print(outpath) - output = self.lcitool_run(args) + output = self._lcitool_run(args) with open(outpath, "w") as f: f.write(output) - def refresh_containers(self): + def _refresh_containers(self): debian_cross = [ "aarch64", "armv6l", @@ -240,39 +240,39 @@ class Application: "mingw64", ] - for host in self.lcitool_get_hosts(): + for host in self._lcitool_get_hosts(): if host.startswith("freebsd-") or host.startswith("macos-"): continue - self.generate_dockerfile(host) + self._generate_dockerfile(host) if host == "fedora-rawhide": for cross in fedora_cross: - self.generate_dockerfile(host, cross) + self._generate_dockerfile(host, cross) if host.startswith("debian-"): for cross in debian_cross: if host == "debian-sid" and cross == "mips": continue - self.generate_dockerfile(host, cross) + self._generate_dockerfile(host, cross) - def refresh_cirrus(self): - for host in self.lcitool_get_hosts(): + def _refresh_cirrus(self): + for host in self._lcitool_get_hosts(): if not (host.startswith("freebsd-") or host.startswith("macos-")): continue - self.generate_vars(host) + self._generate_vars(host) - def action_build(self): - self.make_run(f"ci-build@{self.args.target}") + def _action_build(self): + self._make_run(f"ci-build@{self.args.target}") - def action_test(self): - self.make_run(f"ci-test@{self.args.target}") + def _action_test(self): + self._make_run(f"ci-test@{self.args.target}") - def action_shell(self): - self.make_run(f"ci-shell@{self.args.target}") + def _action_shell(self): + self._make_run(f"ci-shell@{self.args.target}") - def action_list_images(self): + def _action_list_images(self): registry_uri = util.get_registry_uri(self.args.namespace, self.args.gitlab_uri) images = util.get_registry_images(registry_uri) @@ -294,14 +294,14 @@ class Application: print("Available cross-compiler container images:\n") print(spacing + ("\n" + spacing).join(cross)) - def check_stale_images(self): + def _check_stale_images(self): if self.args.check_stale != "yes" or self.args.quiet: return namespace = self.args.namespace gitlab_uri = self.args.gitlab_uri registry_uri = util.get_registry_uri(namespace, gitlab_uri) - lcitool_hosts = self.lcitool_get_hosts() + lcitool_hosts = self._lcitool_get_hosts() stale_images = util.get_registry_stale_images(registry_uri, lcitool_hosts) @@ -326,13 +326,13 @@ You can remove the above images over the API with the following code snippet: You can generate a personal access token here: {gitlab_uri}/-/profile/personal_access_tokens""") - def action_refresh(self): + def _action_refresh(self): # refresh Dockerfiles and vars files - self.refresh_containers() - self.refresh_cirrus() + self._refresh_containers() + self._refresh_cirrus() # check for stale images - self.check_stale_images() + self._check_stale_images() def run(self): self.args.func(self) -- 2.29.2
On Thu, 2021-03-18 at 09:09 +0100, Erik Skultety wrote:
As documented at [1], the common practice wrt to private
s/wrt to/with respect to/
def parse(self): return self.parser.parse_args()
Shouldn't this change be applied to Parser.parser...
def run(self): self.args.func(self)
... and Application.args too? Everything else looks good. -- Andrea Bolognani / Red Hat / Virtualization
On Thu, Mar 18, 2021 at 12:09:19PM +0100, Andrea Bolognani wrote:
On Thu, 2021-03-18 at 09:09 +0100, Erik Skultety wrote:
As documented at [1], the common practice wrt to private
s/wrt to/with respect to/
def parse(self): return self.parser.parse_args()
Shouldn't this change be applied to Parser.parser...
def run(self): self.args.func(self)
... and Application.args too?
Everything else looks good.
Applied. Erik
On Thu, Mar 18, 2021 at 12:09:19PM +0100, Andrea Bolognani wrote:
On Thu, 2021-03-18 at 09:09 +0100, Erik Skultety wrote:
As documented at [1], the common practice wrt to private
s/wrt to/with respect to/
def parse(self): return self.parser.parse_args()
Shouldn't this change be applied to Parser.parser...
def run(self): self.args.func(self)
... and Application.args too?
Everything else looks good.
May I assume your R-b for this patch after having resolved this in v4? Erik
On Fri, 2021-03-19 at 11:36 +0100, Erik Skultety wrote:
On Thu, Mar 18, 2021 at 12:09:19PM +0100, Andrea Bolognani wrote:
On Thu, 2021-03-18 at 09:09 +0100, Erik Skultety wrote:
As documented at [1], the common practice wrt to private
s/wrt to/with respect to/
def parse(self): return self.parser.parse_args()
Shouldn't this change be applied to Parser.parser...
def run(self): self.args.func(self)
... and Application.args too?
Everything else looks good.
May I assume your R-b for this patch after having resolved this in v4?
Sure thing! I thought I had sent an explicit one, but it looks like I failed to do so after all O:-) -- Andrea Bolognani / Red Hat / Virtualization
participants (2)
-
Andrea Bolognani -
Erik Skultety