fix type, documentatuion, duplication, naming

Author: espanov

docs/commands/deploy.md

@@ -135,16 +135,13 @@ usage: gitopscli deploy [-h] --file FILE --values VALUES
 options:
   -h, --help            show this help message and exit
   --file FILE           YAML file path
-  --values VALUES       YAML/JSON object with the YAML path as key and the
-                        desired value as value
+  --values VALUES       YAML/JSON object with the YAML path as key and the desired value as value
   --single-commit [SINGLE_COMMIT]
                         Create only single commit for all updates
   --commit-message COMMIT_MESSAGE
                         Specify exact commit message of deployment commit
-  --username USERNAME   Git username (alternative: GITOPSCLI_USERNAME env
-                        variable)
-  --password PASSWORD   Git password or token (alternative: GITOPSCLI_PASSWORD
-                        env variable)
+  --username USERNAME   Git username (alternative: GITOPSCLI_USERNAME env variable)
+  --password PASSWORD   Git password or token (alternative: GITOPSCLI_PASSWORD env variable)
   --git-user GIT_USER   Git Username
   --git-email GIT_EMAIL
                         Git User Email
@@ -155,18 +152,19 @@ options:
   --git-provider GIT_PROVIDER
                         Git server provider
   --git-provider-url GIT_PROVIDER_URL
-                        Git provider base API URL (e.g.
-                        https://bitbucket.example.tld)
+                        Git provider base API URL (e.g. https://bitbucket.example.tld)
   --create-pr [CREATE_PR]
                         Creates a Pull Request
   --auto-merge [AUTO_MERGE]
-                        Automatically merge the created PR (only valid with
-                        --create-pr)
+                        Automatically merge the created PR (only valid with --create-pr)
   --merge-method MERGE_METHOD
-                        Merge Method (e.g., 'squash', 'rebase', 'merge')
-                        (default: merge)
-  --json
-                        Print a JSON object containing deployment information
+                        Merge Method (e.g., 'squash', 'rebase', 'merge') (default: merge)
+  --json [JSON]         Print a JSON object containing deployment information
+  --pr-labels PR_LABELS
+                        JSON array pr labels (Gitlab, Github supported)
+  --merge-parameters MERGE_PARAMETERS
+                        JSON object pr parameters (only Gitlab supported)
   -v [VERBOSE], --verbose [VERBOSE]
                         Verbose exception logging
+
 ```

gitopscli/cliparser.py

@@ -25,7 +25,6 @@ def parse_args(raw_args: List[str]) -> Tuple[bool, CommandArgs]:
 
     args = vars(parser.parse_args(raw_args))
     args = __deduce_empty_git_provider_from_git_provider_url(args, parser.error)
-    __check_compatibility_args(args, parser.error)
 
     verbose = args.pop("verbose", False)
     command_args = __create_command_args(args)
@@ -114,10 +113,10 @@ def __create_deploy_parser() -> ArgumentParser:
         default=False,
     )
     parser.add_argument(
-        "--pr-labels", help="JSON object pr labels (Gitlab, Github supported)", type=__parse_yaml, default=None
+        "--pr-labels", help="JSON array pr labels (Gitlab, Github supported)", type=__parse_yaml, default=None
     )
     parser.add_argument(
-        "--gitlab-merge-parameters",
+        "--merge-parameters",
         help="JSON object pr parameters (only Gitlab supported)",
         type=__parse_yaml,
         default=None,
@@ -311,15 +310,6 @@ def __deduce_empty_git_provider_from_git_provider_url(
     return updated_args
 
 
-def __check_compatibility_args(args: Dict[str, Any], error: Callable[[str], NoReturn]) -> None:
-    if "git_provider" not in args:
-        return
-    if args.get("pr_labels") and args["git_provider"] == GitProvider.BITBUCKET:
-        error("Cannot use --pr-labels with --git-provider bitbucket-server")
-    if args.get("gitlab_merge_parameters") and args["git_provider"] != GitProvider.GITLAB:
-        error("Can use --gitlab_merge_parameters only with --git-provider gitlab")
-
-
 def __create_command_args(args: Dict[str, Any]) -> CommandArgs:
     args = dict(args)
     command = args.pop("command")

gitopscli/commands/deploy.py

@@ -28,8 +28,8 @@ class Args(GitApiConfig):
         auto_merge: bool
         json: bool
 
-        pr_labels: Any
-        gitlab_merge_parameters: Any
+        pr_labels: Optional[List[str]]
+        merge_parameters: Optional[Any]
         merge_method: Literal["squash", "rebase", "merge"] = "merge"
 
     def __init__(self, args: Args) -> None:
@@ -58,10 +58,8 @@ def execute(self) -> None:
             if self.__args.pr_labels:
                 git_repo_api.add_pull_request_label(pr_id, self.__args.pr_labels)
             if self.__args.auto_merge:
-                if self.__args.gitlab_merge_parameters:
-                    git_repo_api.merge_pull_request_with_parameters(
-                        pr_id, self.__args.gitlab_merge_parameters, self.__args.merge_method
-                    )
+                if self.__args.merge_parameters:
+                    git_repo_api.merge_pull_request(pr_id, self.__args.merge_method, self.__args.merge_parameters)
                 else:
                     git_repo_api.merge_pull_request(pr_id, self.__args.merge_method)
                 git_repo_api.delete_branch(pr_branch)

gitopscli/git_api/bitbucket_git_repo_api_adapter.py

@@ -1,4 +1,4 @@
-from typing import Dict, Any, Optional, Literal
+from typing import List, Dict, Any, Optional, Literal
 import requests
 
 from atlassian import Bitbucket
@@ -74,7 +74,12 @@ def create_pull_request(
             raise GitOpsException(pull_request["errors"][0]["message"])
         return GitRepoApi.PullRequestIdAndUrl(pr_id=pull_request["id"], url=pull_request["links"]["self"][0]["href"])
 
-    def merge_pull_request(self, pr_id: int, merge_method: Literal["squash", "rebase", "merge"] = "merge") -> None:
+    def merge_pull_request(
+        self,
+        pr_id: int,
+        merge_method: Literal["squash", "rebase", "merge"] = "merge",
+        merge_parameters: Dict[str, Any] = None,
+    ) -> None:
         pull_request = self.__bitbucket.get_pullrequest(self.__organisation, self.__repository_name, pr_id)
         self.__bitbucket.merge_pull_request(
             self.__organisation, self.__repository_name, pull_request["id"], pull_request["version"]
@@ -109,13 +114,5 @@ def __get_default_branch(self) -> str:
         default_branch = self.__bitbucket.get_default_branch(self.__organisation, self.__repository_name)
         return str(default_branch["id"])
 
-    def merge_pull_request_with_parameters(
-        self,
-        pr_id: int,
-        gitlab_merge_parameters: Dict[str, Any],
-        merge_method: Literal["squash", "rebase", "merge"] = "merge",
-    ) -> None:
-        pass
-
-    def add_pull_request_label(self, pr_id: int, pr_labels: Dict[str, Any]) -> None:
+    def add_pull_request_label(self, pr_id: int, pr_labels: List[str]) -> None:
         pass

gitopscli/git_api/git_repo_api.py

@@ -1,5 +1,5 @@
 from abc import ABCMeta, abstractmethod
-from typing import Any, Dict, NamedTuple, Optional, Literal
+from typing import Any, List, Dict, NamedTuple, Optional, Literal
 
 
 class GitRepoApi(metaclass=ABCMeta):
@@ -32,7 +32,12 @@ def create_pull_request(
         ...
 
     @abstractmethod
-    def merge_pull_request(self, pr_id: int, merge_method: Literal["squash", "rebase", "merge"] = "merge") -> None:
+    def merge_pull_request(
+        self,
+        pr_id: int,
+        merge_method: Literal["squash", "rebase", "merge"] = "merge",
+        merge_parameters: Dict[str, Any] = None,
+    ) -> None:
         ...
 
     @abstractmethod
@@ -52,14 +57,5 @@ def get_pull_request_branch(self, pr_id: int) -> str:
         ...
 
     @abstractmethod
-    def add_pull_request_label(self, pr_id: int, pr_labels: Dict[str, Any]) -> None:
-        ...
-
-    @abstractmethod
-    def merge_pull_request_with_parameters(
-        self,
-        pr_id: int,
-        gitlab_merge_parameters: Dict[str, Any],
-        merge_method: Literal["squash", "rebase", "merge"] = "merge",
-    ) -> None:
+    def add_pull_request_label(self, pr_id: int, pr_labels: List[str]) -> None:
         ...

gitopscli/git_api/git_repo_api_logging_proxy.py

@@ -1,5 +1,5 @@
 import logging
-from typing import Any, Dict, Optional, Literal
+from typing import Any, List, Dict, Optional, Literal
 from .git_repo_api import GitRepoApi
 
 
@@ -28,7 +28,12 @@ def create_pull_request(
         logging.info("Creating pull request from '%s' to '%s' with title: %s", from_branch, to_branch, title)
         return self.__api.create_pull_request(from_branch, to_branch, title, description)
 
-    def merge_pull_request(self, pr_id: int, merge_method: Literal["squash", "rebase", "merge"] = "merge") -> None:
+    def merge_pull_request(
+        self,
+        pr_id: int,
+        merge_method: Literal["squash", "rebase", "merge"] = "merge",
+        merge_parameters: Dict[str, Any] = None,
+    ) -> None:
         logging.info("Merging pull request %s", pr_id)
         self.__api.merge_pull_request(pr_id, merge_method=merge_method)
 
@@ -51,15 +56,6 @@ def get_branch_head_hash(self, branch: str) -> str:
     def get_pull_request_branch(self, pr_id: int) -> str:
         return self.__api.get_pull_request_branch(pr_id)
 
-    def add_pull_request_label(self, pr_id: int, pr_labels: Dict[str, Any]) -> None:
+    def add_pull_request_label(self, pr_id: int, pr_labels: List[str]) -> None:
         logging.info("Adding labels for pull request %s with content: %s", pr_id, pr_labels)
         self.__api.add_pull_request_label(pr_id, pr_labels)
-
-    def merge_pull_request_with_parameters(
-        self,
-        pr_id: int,
-        gitlab_merge_parameters: Dict[str, Any],
-        merge_method: Literal["squash", "rebase", "merge"] = "merge",
-    ) -> None:
-        logging.info("Merging pull request %s with parameters %s", pr_id, gitlab_merge_parameters)
-        self.__api.merge_pull_request_with_parameters(pr_id, gitlab_merge_parameters, merge_method=merge_method)

gitopscli/git_api/github_git_repo_api_adapter.py

@@ -45,7 +45,12 @@ def create_pull_request(
         pull_request = repo.create_pull(title=title, body=description, head=from_branch, base=to_branch)
         return GitRepoApi.PullRequestIdAndUrl(pr_id=pull_request.number, url=pull_request.html_url)
 
-    def merge_pull_request(self, pr_id: int, merge_method: Literal["squash", "rebase", "merge"] = "merge") -> None:
+    def merge_pull_request(
+        self,
+        pr_id: int,
+        merge_method: Literal["squash", "rebase", "merge"] = "merge",
+        merge_parameters: Dict[str, Any] = None,
+    ) -> None:
         pull_request = self.__get_pull_request(pr_id)
         pull_request.merge(merge_method=merge_method)
 
@@ -92,11 +97,3 @@ def __get_repo(self) -> Repository.Repository:
     def add_pull_request_label(self, pr_id: int, pr_labels: Union[str, Any]) -> None:
         pull_request = self.__get_pull_request(pr_id)
         pull_request.set_labels(pr_labels)
-
-    def merge_pull_request_with_parameters(
-        self,
-        pr_id: int,
-        gitlab_merge_parameters: Dict[str, Any],
-        merge_method: Literal["squash", "rebase", "merge"] = "merge",
-    ) -> None:
-        pass

gitopscli/git_api/gitlab_git_repo_api_adapter.py

@@ -1,4 +1,4 @@
-from typing import Any, Dict, Optional, Literal
+from typing import Any, List, Dict, Optional, Literal
 import logging
 import time
 import requests
@@ -59,16 +59,21 @@ def create_pull_request(
         )
         return GitRepoApi.PullRequestIdAndUrl(pr_id=merge_request.iid, url=merge_request.web_url)
 
-    def merge_pull_request(self, pr_id: int, merge_method: Literal["squash", "rebase", "merge"] = "merge") -> None:
+    def merge_pull_request(
+        self,
+        pr_id: int,
+        merge_method: Literal["squash", "rebase", "merge"] = "merge",
+        merge_parameters: Dict[str, Any] = None,
+    ) -> None:
         merge_request = self.__project.mergerequests.get(pr_id)
 
         max_retries = MAX_MERGE_RETRIES
         while max_retries > 0:
             try:
                 if merge_method == "rebase":
-                    merge_request.rebase()
+                    merge_request.rebase(merge_parameters)
                     return
-                merge_request.merge()
+                merge_request.merge(merge_parameters)
                 return
             except gitlab.exceptions.GitlabMRClosedError as ex:
                 # "Branch cannot be merged" error can occur if the server
@@ -103,34 +108,7 @@ def __get_default_branch(self) -> str:
             raise GitOpsException("Default branch does not exist")
         return str(default_branch.name)
 
-    def add_pull_request_label(self, pr_id: int, pr_labels: Dict[str, Any]) -> None:
+    def add_pull_request_label(self, pr_id: int, pr_labels: List[str]) -> None:
         merge_request = self.__project.mergerequests.get(pr_id)
         merge_request.labels = pr_labels
         merge_request.save()
-
-    def merge_pull_request_with_parameters(
-        self,
-        pr_id: int,
-        gitlab_merge_parameters: Dict[str, Any],
-        merge_method: Literal["squash", "rebase", "merge"] = "merge",
-    ) -> None:
-        merge_request = self.__project.mergerequests.get(pr_id)
-
-        max_retries = MAX_MERGE_RETRIES
-        while max_retries > 0:
-            try:
-                if merge_method == "rebase":
-                    merge_request.rebase(gitlab_merge_parameters)
-                    return
-                merge_request.merge(gitlab_merge_parameters)
-                return
-            except gitlab.exceptions.GitlabMRClosedError as ex:
-                # "Branch cannot be merged" error can occur if the server
-                # is still processing the merge request internally
-                max_retries -= 1
-                logging.warning(
-                    "Retry merging pull request. Attempts: (%s/%s)", MAX_MERGE_RETRIES - max_retries, MAX_MERGE_RETRIES
-                )
-                if max_retries == 0:
-                    raise GitOpsException("Error merging pull request: 'Branch cannot be merged'") from ex
-                time.sleep(2.5)