commit 1 all good
one inline comment on commit 2 - log is obsolete next to status.maintenance in
some places
We should also look to not include the webcontrol/ and autopkgtest-cloud/
directories in the charms, though we will do this work in ADT-664. This should
speed up `charmcraft pack`.
Other than that, LGTM
Diff comments:
> diff --git
> a/charms/focal/autopkgtest-cloud-worker/reactive/autopkgtest_cloud_worker.py
> b/charms/focal/autopkgtest-cloud-worker/reactive/autopkgtest_cloud_worker.py
> index f2a8ab7..bd6e4e4 100644
> ---
> a/charms/focal/autopkgtest-cloud-worker/reactive/autopkgtest_cloud_worker.py
> +++
> b/charms/focal/autopkgtest-cloud-worker/reactive/autopkgtest_cloud_worker.py
> @@ -149,6 +161,26 @@ def clone_autopkgtest():
> log("autopkgtest already cloned", "INFO")
>
>
> +@when("apt.installed.git")
> +@when_not("autopkgtest.autopkgtest-cloud_cloned")
> +def clone_autopkgtest_cloud():
> + status.maintenance("Cloning autopkgtest-cloud")
> + log("Cloning autopkgtest-cloud", "INFO")
this isn't needed, line above does the same thing - your decision as to whether
you want to or not
> + with UnixUser("ubuntu"):
> + try:
> + pygit2.clone_repository(
> + AUTOPKGTEST_CLOUD_CLONE_LOCATION,
> + AUTOPKGTEST_CLOUD_GIT_LOCATION,
> + checkout_branch=AUTOPKGTEST_CLOUD_CLONE_BRANCH,
> + )
> + status.maintenance("autopkgtest-cloud cloned")
> + log("autopkgtest-cloud cloned", "INFO")
> + set_flag("autopkgtest.autopkgtest-cloud_cloned")
> + except ValueError:
> + # exists
> + log("autopkgtest-cloud already cloned", "INFO")
> +
> +
> @when_all(
> "autopkgtest.autopkgtest_cloud_symlinked",
> "autopkgtest.rabbitmq-configured",
> diff --git a/charms/focal/autopkgtest-web/lib/utils.py
> b/charms/focal/autopkgtest-web/lib/utils.py
> new file mode 100644
> index 0000000..4f1316c
> --- /dev/null
> +++ b/charms/focal/autopkgtest-web/lib/utils.py
> @@ -0,0 +1,55 @@
> +# pylint: disable=missing-module-docstring, missing-class-docstring,
> missing-function-docstring
> +import os
> +import pwd
> +
> +from charmhelpers.core.hookenv import log
> +
> +
> +# class UnixUser(object):
leftover comment
> +class UnixUser:
> + def __init__(self, username):
> + self.username = username
> + pwnam = pwd.getpwnam(username)
> + self.uid = pwnam.pw_uid
> + self.gid = pwnam.pw_gid
> +
> + def __enter__(self):
> + log(
> + "Raising UID to {uid} and GID to {gid}".format(
> + uid=self.uid, gid=self.gid
> + ),
> + "INFO",
> + )
> + os.setresgid(self.gid, self.gid, os.getgid())
> + os.setresuid(self.uid, self.uid, os.getuid())
> +
> + def __exit__(self, exc_type, exc_val, exc_tb):
> + _, _, suid = os.getresuid()
> + _, _, sgid = os.getresgid()
> + log(
> + "Restoring UID to {suid} and GID to {sgid}".format(
> + suid=suid, sgid=sgid
> + ),
> + "INFO",
> + )
> + os.setresuid(suid, suid, suid)
> + os.setresgid(sgid, sgid, sgid)
> +
> +
> +def pull(repository, branch):
> + """
> + This will do a sort of git fetch origin && git reset --hard origin/master
> + This raises pygit2.GitError whenever the tree is not clean, preventing
> to loose cowboys.
> + """
> + origin = [
> + remote for remote in repository.remotes if remote.name == "origin"
> + ][0]
> + origin.fetch()
> + remote_master_id = repository.lookup_reference(
> + f"refs/remotes/origin/{branch}"
> + ).target
> + repository.checkout_tree(repository.get(remote_master_id))
> + master_ref = repository.lookup_reference(f"refs/heads/{branch}")
> + master_ref.set_target(remote_master_id)
> + repository.head.set_target(remote_master_id)
> + log(f"Updated repository {repository} to {remote_master_id}", "INFO")
--
https://code.launchpad.net/~hyask/autopkgtest-cloud/+git/autopkgtest-cloud/+merge/471159
Your team Canonical's Ubuntu QA is requested to review the proposed merge of
~hyask/autopkgtest-cloud:skia/git_clone_in_charms into autopkgtest-cloud:master.
--
Mailing list: https://launchpad.net/~canonical-ubuntu-qa
Post to : canonical-ubuntu-qa@lists.launchpad.net
Unsubscribe : https://launchpad.net/~canonical-ubuntu-qa
More help : https://help.launchpad.net/ListHelp
