launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #30522
Re: [Merge] ~bowenfan/turnip:SN2052-add_ref_creation_to_turnip_api into turnip:master
Diff comments:
> diff --git a/turnip/api/store.py b/turnip/api/store.py
> index ae897c2..9475bce 100644
> --- a/turnip/api/store.py
> +++ b/turnip/api/store.py
> @@ -555,6 +557,24 @@ def get_ref(repo_store, repo_name, ref):
> return ref_obj
>
>
> +def write_lightweight_tag(repo_store, repo_name, commit_sha1, tag_name):
This is nitpicky, but I think the signature should look like (repo_store, repo_name, ref_name, commit_sha1), given the existing signature for `get_ref(repo_store, repo_name, ref)`
> + ref_base = "refs/tags/"
> + ref_name = ref_base + tag_name
> + with open_repo(repo_store, repo_name) as repo:
> + try:
> + repo.create_reference(ref_name, commit_sha1)
> + except (AlreadyExistsError, InvalidSpecError):
> + raise
> +
> +
> +def create_branch(repo_store, repo_name, commit, branch_name):
Similar to the above comment, I think the signature should be create_branch(repo_store, repo_name, branch_name, commit)
> + with open_repo(repo_store, repo_name) as repo:
> + try:
> + repo.create_branch(branch_name, commit)
> + except (AlreadyExistsError, InvalidSpecError):
> + raise
> +
> +
> def get_common_ancestor_diff(
> repo_store, repo_name, sha1_target, sha1_source, context_lines=3
> ):
> diff --git a/turnip/api/views.py b/turnip/api/views.py
> index 5014634..6ee9833 100644
> --- a/turnip/api/views.py
> +++ b/turnip/api/views.py
> @@ -265,6 +265,76 @@ class RefAPI(BaseAPI):
> return exc.HTTPNotFound() # 404
>
> @validate_path
> + def collection_post(self, repo_store, repo_name):
> + """Create a lightweight tag or a branch against a commit.
> +
> + The JSON request dictionary should contain the following keys:
> + {commit_sha1} must be the full 40-char SHA1 hash of the commit to tag
> + or set as a branch head.
> + """
> + json_data = extract_cstruct(self.request)["body"]
> + commit_sha1 = json_data.get("commit_sha1")
> + ref_type = json_data.get("ref_type")
> + ref_name = json_data.get("ref_name")
> +
> + repo_path = os.path.join(repo_store, repo_name)
> + if not os.path.exists(repo_path):
> + self.request.errors.add(
> + "body", "name", "repository does not exist"
> + )
> + raise exc.HTTPNotFound()
I think the check above might be redundant given the @validate_path decorator above
> +
> + if not commit_sha1:
> + self.request.errors.add(
> + "body", "commit_sha1", "commit_sha1 for ref target is missing"
> + )
> + return
Pooling all the errors for the keys missing from the body over here would be good for failing fast (before checking if the commit exists).
> + try:
> + commit = store.get_commit(repo_store, repo_name, commit_sha1)
> + except GitError:
> + self.request.errors.add(
> + "body", "commit_sha1", "no commit found for commit_sha1 given"
> + )
> + raise exc.HTTPNotFound()
> + if not ref_type or ref_type not in {"lightweight_tag", "branch"}:
> + self.request.errors.add(
> + "body",
> + "ref_type",
> + "must specify ref type (lightweight_tag or branch)",
> + )
> + return
> + if not ref_name:
> + self.request.errors.add(
> + "body", "ref_name", "must specify ref name"
> + )
> + return
> +
> + try:
> + if ref_type == "lightweight_tag":
> + store.write_lightweight_tag(
> + repo_store, repo_name, commit_sha1, ref_name
> + )
> + elif ref_type == "branch":
> + store.create_branch(repo_store, repo_name, commit, ref_name)
> + except AlreadyExistsError:
> + self.request.errors.add(
> + "body",
> + "ref_name",
> + "tag or branch name already exists",
> + )
> + self.request.errors.status = 409
> + return
> + except InvalidSpecError:
> + self.request.errors.add(
> + "body",
> + "ref_name",
> + "ensure tag or branch name is valid per git-check-ref-format",
> + )
> + return
> + else:
> + return Response(status=201)
> +
> + @validate_path
> def get(self, repo_store, repo_name):
> ref = "refs/" + self.request.matchdict["ref"]
> try:
--
https://code.launchpad.net/~bowenfan/turnip/+git/turnip/+merge/452409
Your team Launchpad code reviewers is requested to review the proposed merge of ~bowenfan/turnip:SN2052-add_ref_creation_to_turnip_api into turnip:master.
Follow ups