nit(scm): group functions by class they come from in main integration class (#76240)

Author: cathteng

src/sentry/integrations/bitbucket/integration.py

@@ -90,13 +90,13 @@ def integration_name(self) -> str:
     def get_client(self):
         return BitbucketApiClient(integration=self.model)
 
-    @property
-    def username(self):
-        return self.model.name
+    # IntegrationInstallation methods
 
     def error_message_from_json(self, data):
         return data.get("error", {}).get("message", "unknown error")
 
+    # RepositoryIntegration methods
+
     def get_repositories(self, query=None):
         username = self.model.metadata.get("uuid", self.username)
         if not query:
@@ -156,6 +156,12 @@ def extract_source_path_from_source_url(self, repo: Repository, url: str) -> str
         _, _, source_path = url.partition("/")
         return source_path
 
+    # Bitbucket only methods
+
+    @property
+    def username(self):
+        return self.model.name
+
 
 class BitbucketIntegrationProvider(IntegrationProvider):
     key = "bitbucket"

src/sentry/integrations/bitbucket_server/integration.py

@@ -249,13 +249,13 @@ def get_client(self):
             identity=self.default_identity,
         )
 
-    @property
-    def username(self):
-        return self.model.name
+    # IntegrationInstallation methods
 
     def error_message_from_json(self, data):
         return data.get("error", {}).get("message", "unknown error")
 
+    # RepositoryIntegration methods
+
     def get_repositories(self, query=None):
         if not query:
             resp = self.get_client().get_repos()
@@ -310,6 +310,12 @@ def extract_branch_from_source_url(self, repo: Repository, url: str) -> str:
     def extract_source_path_from_source_url(self, repo: Repository, url: str) -> str:
         raise IntegrationFeatureNotImplementedError
 
+    # Bitbucket Server only methods
+
+    @property
+    def username(self):
+        return self.model.name
+
 
 class BitbucketServerIntegrationProvider(IntegrationProvider):
     key = "bitbucket_server"

src/sentry/integrations/github/integration.py

@@ -268,7 +268,7 @@ def has_repo_access(self, repo: RpcRepository) -> bool:
             return False
         return True
 
-    # for derive code mappings (TODO: define in an ABC)
+    # for derive code mappings - TODO(cathy): define in an ABC
     def get_trees_for_org(self, cache_seconds: int = 3600 * 24) -> dict[str, RepoTree]:
         trees: dict[str, RepoTree] = {}
         domain_name = self.model.metadata["domain_name"]
@@ -291,7 +291,7 @@ def get_trees_for_org(self, cache_seconds: int = 3600 * 24) -> dict[str, RepoTre
 
         return trees
 
-    # TODO: define in issue ABC
+    # TODO(cathy): define in issue ABC
     def search_issues(self, query: str) -> Mapping[str, Sequence[Mapping[str, Any]]]:
         return self.get_client().search_issues(query)
 

src/sentry/integrations/github_enterprise/integration.py

@@ -154,6 +154,19 @@ def get_client(self):
             org_integration_id=self.org_integration.id,
         )
 
+    # IntegrationInstallation methods
+
+    def message_from_error(self, exc):
+        if isinstance(exc, ApiError):
+            message = API_ERRORS.get(exc.code)
+            if message is None:
+                message = exc.json.get("message", "unknown error") if exc.json else "unknown error"
+            return f"Error Communicating with GitHub Enterprise (HTTP {exc.code}): {message}"
+        else:
+            return ERR_INTERNAL
+
+    # RepositoryIntegration methods
+
     def get_repositories(self, query=None):
         if not query:
             return [
@@ -197,15 +210,6 @@ def has_repo_access(self, repo: RpcRepository) -> bool:
         # TODO: define this, used to migrate repositories
         return False
 
-    def message_from_error(self, exc):
-        if isinstance(exc, ApiError):
-            message = API_ERRORS.get(exc.code)
-            if message is None:
-                message = exc.json.get("message", "unknown error") if exc.json else "unknown error"
-            return f"Error Communicating with GitHub Enterprise (HTTP {exc.code}): {message}"
-        else:
-            return ERR_INTERNAL
-
 
 class InstallationForm(forms.Form):
     url = forms.CharField(

src/sentry/integrations/gitlab/integration.py

@@ -103,9 +103,6 @@ def __init__(self, *args, **kwargs):
     def integration_name(self) -> str:
         return "gitlab"
 
-    def get_group_id(self):
-        return self.model.metadata["group_id"]
-
     def get_client(self):
         if self.default_identity is None:
             try:
@@ -115,6 +112,22 @@ def get_client(self):
 
         return GitLabApiClient(self)
 
+    # IntegrationInstallation methods
+    def error_message_from_json(self, data):
+        """
+        Extract error messages from gitlab API errors.
+        Generic errors come in the `error` key while validation errors
+        are generally in `message`.
+
+        See https://docs.gitlab.com/ee/api/#data-validation-and-error-reporting
+        """
+        if "message" in data:
+            return data["message"]
+        if "error" in data:
+            return data["error"]
+
+    # RepositoryIntegration methods
+
     def has_repo_access(self, repo: RpcRepository) -> bool:
         # TODO: define this, used to migrate repositories
         return False
@@ -148,28 +161,21 @@ def extract_source_path_from_source_url(self, repo: Repository, url: str) -> str
         _, _, source_path = url.partition("/")
         return source_path
 
+    # Gitlab only functions
+
+    def get_group_id(self):
+        return self.model.metadata["group_id"]
+
     def search_projects(self, query):
         client = self.get_client()
         group_id = self.get_group_id()
         return client.search_projects(group_id, query)
 
+    # TODO(cathy): define in issue ABC
     def search_issues(self, project_id, query, iids):
         client = self.get_client()
         return client.search_project_issues(project_id, query, iids)
 
-    def error_message_from_json(self, data):
-        """
-        Extract error messages from gitlab API errors.
-        Generic errors come in the `error` key while validation errors
-        are generally in `message`.
-
-        See https://docs.gitlab.com/ee/api/#data-validation-and-error-reporting
-        """
-        if "message" in data:
-            return data["message"]
-        if "error" in data:
-            return data["error"]
-
 
 class InstallationForm(forms.Form):
     url = forms.CharField(

src/sentry/integrations/vsts/integration.py

@@ -131,47 +131,12 @@ def __init__(self, *args: Any, **kwargs: Any) -> None:
     def integration_name(self) -> str:
         return "vsts"
 
-    def all_repos_migrated(self) -> bool:
-        return not self.get_unmigratable_repositories()
-
-    def get_repositories(self, query: str | None = None) -> Sequence[Mapping[str, str]]:
-        try:
-            repos = self.get_client().get_repos()
-        except (ApiError, IdentityNotValid) as e:
-            raise IntegrationError(self.message_from_error(e))
-        data = []
-        for repo in repos["value"]:
-            data.append(
-                {
-                    "name": "{}/{}".format(repo["project"]["name"], repo["name"]),
-                    "identifier": repo["id"],
-                }
-            )
-        return data
-
-    def get_unmigratable_repositories(self) -> list[RpcRepository]:
-        repos = repository_service.get_repositories(
-            organization_id=self.organization_id, providers=["visualstudio"]
-        )
-        identifiers_to_exclude = {r["identifier"] for r in self.get_repositories()}
-        return [repo for repo in repos if repo.external_id not in identifiers_to_exclude]
-
-    def has_repo_access(self, repo: RpcRepository) -> bool:
-        client = self.get_client()
-        try:
-            # since we don't actually use webhooks for vsts commits,
-            # just verify repo access
-            client.get_repo(repo.config["name"], project=repo.config["project"])
-        except (ApiError, IdentityNotValid):
-            return False
-        return True
-
     def get_client(self) -> VstsApiClient:
         base_url = self.instance
         if SiloMode.get_current_mode() != SiloMode.REGION:
             if self.default_identity is None:
                 self.default_identity = self.get_default_identity()
-            self.check_domain_name(self.default_identity)
+            self._check_domain_name(self.default_identity)
 
         if self.org_integration is None:
             raise Exception("self.org_integration is not defined")
@@ -184,15 +149,7 @@ def get_client(self) -> VstsApiClient:
             identity_id=self.org_integration.default_auth_id,
         )
 
-    def check_domain_name(self, default_identity: RpcIdentity) -> None:
-        if re.match("^https://.+/$", self.model.metadata["domain_name"]):
-            return
-
-        base_url = VstsIntegrationProvider.get_base_url(
-            default_identity.data["access_token"], self.model.external_id
-        )
-        self.model.metadata["domain_name"] = base_url
-        self.model.save()
+    # IntegrationInstallation methods
 
     def get_organization_config(self) -> Sequence[Mapping[str, Any]]:
         client = self.get_client()
@@ -333,6 +290,40 @@ def get_config_data(self) -> Mapping[str, Any]:
         config["sync_status_forward"] = sync_status_forward
         return config
 
+    # RepositoryIntegration methods
+
+    def get_repositories(self, query: str | None = None) -> Sequence[Mapping[str, str]]:
+        try:
+            repos = self.get_client().get_repos()
+        except (ApiError, IdentityNotValid) as e:
+            raise IntegrationError(self.message_from_error(e))
+        data = []
+        for repo in repos["value"]:
+            data.append(
+                {
+                    "name": "{}/{}".format(repo["project"]["name"], repo["name"]),
+                    "identifier": repo["id"],
+                }
+            )
+        return data
+
+    def get_unmigratable_repositories(self) -> list[RpcRepository]:
+        repos = repository_service.get_repositories(
+            organization_id=self.organization_id, providers=["visualstudio"]
+        )
+        identifiers_to_exclude = {r["identifier"] for r in self.get_repositories()}
+        return [repo for repo in repos if repo.external_id not in identifiers_to_exclude]
+
+    def has_repo_access(self, repo: RpcRepository) -> bool:
+        client = self.get_client()
+        try:
+            # since we don't actually use webhooks for vsts commits,
+            # just verify repo access
+            client.get_repo(repo.config["name"], project=repo.config["project"])
+        except (ApiError, IdentityNotValid):
+            return False
+        return True
+
     def source_url_matches(self, url: str) -> bool:
         return url.startswith(self.model.metadata["domain_name"])
 
@@ -362,6 +353,18 @@ def extract_source_path_from_source_url(self, repo: Repository, url: str) -> str
             return qs["path"][0].lstrip("/")
         return ""
 
+    # Azure DevOps only methods
+
+    def _check_domain_name(self, default_identity: RpcIdentity) -> None:
+        if re.match("^https://.+/$", self.model.metadata["domain_name"]):
+            return
+
+        base_url = VstsIntegrationProvider.get_base_url(
+            default_identity.data["access_token"], self.model.external_id
+        )
+        self.model.metadata["domain_name"] = base_url
+        self.model.save()
+
     @property
     def instance(self) -> str:
         return self.model.metadata["domain_name"]