Either we have different blacks installed, or you have yours configured to use a long line length.
I've updated the branch to satisfy almost all of your comments. Squashed and pushed as the policy dictates for LP MRs. Diff comments: > diff --git a/add_private_ppa.py b/add_private_ppa.py > new file mode 100644 > index 0000000..d8228bd > --- /dev/null > +++ b/add_private_ppa.py > @@ -0,0 +1,240 @@ > +#!/usr/bin/env python3 > +# > +# Copyright 2023 Canonical Ltd. > +# Written by: > +# Maciej Kisielewski <[email protected]> > + > +""" > +This program adds a private PPA to the system. > + > +It does so by creating a credentials file that holds the login and password, > +so they are not stored in plaintext in the URIs in the sources.list file. > + > +Then the URL (without the login and password) is added to the sources.list > file. > + > +Finally, the PPA's key is added to the system. > +""" > + > +import argparse > +import logging > +import os > +import subprocess > +import textwrap > +from urllib.parse import urlparse > + > +logging.basicConfig(level=logging.INFO) > +logging.getLogger().name = os.path.basename(__file__) The default logger is always there. I really couldn't see what making another instance of a logger would solve. Also the setup of the local logger to achieve the same thing would be longer (the prefix of ERROR:filename) - unless there's a nice one-liner to do it, if there is, please do share! > + > + > +def guess_ubuntu_codename() -> str: > + """ > + Guess the Ubuntu codename. > + > + The codename is guessed by running the lsb_release command. > + """ > + try: > + logging.info("Guessing Ubuntu codename...") > + codename = subprocess.check_output( > + ["lsb_release", "--codename", "--short"], universal_newlines=True > + ).strip() > + logging.info("Ubuntu codename guessed: %s", codename) > + return codename > + except FileNotFoundError: > + logging.error("lsb_release not found!") > + raise SystemExit(1) I like bubbling up error messages in SystemError as well. Originally the messages where multi-line, this is why I separated them, but I'll change it back to just raising. Btw. I don't understand the comment about this not being an error. You mean that this is a more serious thing? With this complexity it's only cosmetic, and ERROR is imho better to pick out/understand. > + except (subprocess.CalledProcessError, OSError) as e: > + logging.error("Problem encountered when running lsb_release: %s", e) > + raise SystemExit(1) > + > + > +def trust_the_key(key: str) -> None: > + """ > + Trust the key. > + > + The key is trusted by running the apt-key command. > + """ > + try: > + logging.info("Trusting the key...") > + subprocess.check_call(["apt-key", "add", key]) > + logging.info("Key trusted.") > + except FileNotFoundError: > + logging.error("apt-key not found!") > + raise SystemExit(1) > + except subprocess.CalledProcessError as e: > + logging.error("Problem encountered when running apt-key: %s", e) > + raise SystemExit(1) > + > + > +def slugify_name(name: str) -> str: > + """ > + Slugify a name, so it can be used as a filename. > + The characters that are not allowed in filenames are replaced with a > dash. > + The characters being replaced are: '/', '\', ':', '*', '?', '"', '<', > '>', '|', and space. > + > + >>> slugify_name("") > + '' > + >>> slugify_name("a") > + 'a' > + >>> slugify_name("a/b") > + 'a-b' > + >>> slugify_name("a\\\\b") # double escaping because of doctest > + 'a-b' > + >>> slugify_name("a:b") > + 'a-b' > + >>> slugify_name("a*b") > + 'a-b' > + >>> slugify_name("a?b") > + 'a-b' > + >>> slugify_name('a"b') > + 'a-b' > + >>> slugify_name("a<b") > + 'a-b' > + >>> slugify_name("a>b") > + 'a-b' > + >>> slugify_name("a|b") > + 'a-b' > + >>> slugify_name("a b") > + 'a-b' > + >>> slugify_name(r"a/b\\:*?\\"<>| ") > + 'a-b----------' > + """ > + return ( I think the most common special char in URLs is '%' and slugifying it would unnecessarily obfuscate the source, this is why I rejected regex sub early on. I also like to dodge the regexes as much as possible to make it obvious for the next person :) > + name.replace("/", "-") > + .replace("\\", "-") > + .replace(":", "-") > + .replace("*", "-") > + .replace("?", "-") > + .replace('"', "-") > + .replace("<", "-") > + .replace(">", "-") > + .replace("|", "-") > + .replace(" ", "-") > + ) > + > + > +def create_apt_auth_file(ppa: str, login: str, password: str) -> None: > + """ > + Create a credentials file for the PPA. > + > + The file will be named after the PPA's name, with the login and password > + appended to it, and will be placed in the /etc/apt/auth.conf.d/ > directory. > + """ > + ppa_name = slugify_name(extract_ppa_name(ppa)) > + auth_file_path = "/etc/apt/auth.conf.d/{}.conf".format(ppa_name) removed the first one; renamed the second one to auth_file_path > + contents = textwrap.dedent( > + """ > + machine {} > + login {} > + password {} > + """ > + ).format(ppa, login, password) > + > + auth_file = "/etc/apt/auth.conf.d/ppa-{}.conf".format(ppa_name) > + if os.path.exists(auth_file): > + logging.warning("Credentials file already exists: %s", auth_file) > + logging.warning("Not overwriting it.") > + else: > + with open(auth_file, "w") as f: > + f.write(contents) > + logging.info("Created credentials file: %s", auth_file) > + > + > +def extract_ppa_name(url: str) -> str: > + """ > + Extracts the PPA address from a URL. > + > + >>> > extract_ppa_name('https://private-ppa.launchpadcontent.net/test/test-path') > + 'test/test-path' > + > + >>> > extract_ppa_name('https://private-ppa.launchpadcontent.net/singlepath') > + 'singlepath' > + > + >>> extract_ppa_name('https://private-ppa.launchpadcontent.net/') > + '' > + > + >>> extract_ppa_name('https://private-ppa.launchpadcontent.net') yeah, makes sense > + '' > + > + >>> extract_ppa_name('not-a-url') > + 'not-a-url' ditto > + """ > + parsed_url = urlparse(url) > + path = parsed_url.path > + # Remove initial slash > + if path.startswith("/"): > + path = path[1:] > + return path > + > + > +def add_ppa_to_sources_list(ppa: str) -> None: > + """ > + Add the PPA to the sources.list file. > + > + The PPA's URL will be added to the sources.list file, with the login and > + password replaced with the name of the credentials file. > + """ > + ppa_name = slugify_name(extract_ppa_name(ppa)) > + sources_list_file = "/etc/apt/sources.list.d/{}.list".format(ppa_name) > + release_codename = guess_ubuntu_codename() > + contents = textwrap.dedent( > + """ > + deb {ppa} {release_codename} main > + deb-src {ppa} {release_codename} main > + """ > + ).format(ppa=ppa, release_codename=release_codename) > + > + if os.path.exists(sources_list_file): > + logging.warning( > + "Sources list file already exists: %s", sources_list_file > + ) > + logging.warning("Not overwriting it.") > + else: > + with open(sources_list_file, "w") as f: > + f.write(contents) > + logging.info("Created sources list file: %s", sources_list_file) > + > + > +def add_ppa_key(key: str) -> None: > + """ > + Add the PPA's key to the system. > + > + The PPA's key will be added to the system by running the apt-key command. > + """ > + try: > + cmd = [ > + "apt-key", > + "adv", > + "--keyserver", > + "keyserver.ubuntu.com", > + "--recv-keys", > + key, > + ] > + > + logging.info("Adding the PPA's key using apt-key...") > + subprocess.check_call(cmd) > + logging.info("PPA's key added.") > + except FileNotFoundError: > + logging.error("apt-key not found!") > + raise SystemExit(1) > + except subprocess.CalledProcessError as e: > + logging.error("Problem encountered when running apt-key: %s", e) > + raise SystemExit(1) > + > + > +def main() -> None: > + print(guess_ubuntu_codename()) oops done > + parser = argparse.ArgumentParser( > + description="Add a private PPA to the system." > + ) > + parser.add_argument("ppa", help="The URL of the PPA to add.") > + parser.add_argument("login", help="The login to use for the PPA.") > + parser.add_argument("password", help="The password to use for the PPA.") > + parser.add_argument("key", help="PPA's key to add.") > + args = parser.parse_args() > + create_apt_auth_file(args.ppa, args.login, args.password) > + add_ppa_to_sources_list(args.ppa) > + add_ppa_key(args.key) great idea, thanks! > + > + > +if __name__ == "__main__": > + main() -- https://code.launchpad.net/~kissiel/hwcert-jenkins-tools/+git/hwcert-jenkins-tools/+merge/447532 Your team Canonical Hardware Certification is subscribed to branch hwcert-jenkins-tools:master. -- Mailing list: https://launchpad.net/~canonical-hw-cert Post to : [email protected] Unsubscribe : https://launchpad.net/~canonical-hw-cert More help : https://help.launchpad.net/ListHelp
