Fix code quality issues and Pydantic v2 migration

- Remove unreachable code in github_provider.py after exception raise - Fix write_file() to always use PUT method for consistency - Fix get_repository_url() to strip /api/v3 from GitHub Enterprise URLs - Fix exception handling in gitlab_provider.py to catch RequestException - Migrate app.py to Pydantic v2 (dict() -> model_dump()) - Remove 11 redundant import statements across multiple modules - Add top-level traceback import to app.py - Fix type annotations (_request() returns Any, not Dict) - Migrate FileContent and TemplateConfig to Pydantic v2 model_config - Remove 6 dead dependencies (PyGithub, python-gitlab, etc.) - Add pydantic>=2.0 to setup.py - Rewrite all test files to use template_automation API - Add __init__.py files for proper package structure - All 14 tests now passing
CSVD · Feb 6, 2026 · 44aa5a7 · 44aa5a7
1 parent 83a0b96
commit 44aa5a7
Show file tree

Hide file tree

Showing 13 changed files with 298 additions and 310 deletions.
diff --git a/requirements.txt b/requirements.txt
@@ -1,12 +1,6 @@
 # Core dependencies
-PyGithub>=1.59.0
 pydantic>=2.0.0
 boto3>=1.26.0
 requests>=2.28.0
 jinja2>=3.1.0
 typing_extensions>=4.4.0
-pynacl>=1.5.0       # Required by PyGithub for cryptography
-cryptography>=44.0.0 # Required by PyGithub for auth
-pyjwt[crypto]>=2.10.0   # Required by PyGithub for JWT support
-deprecated>=1.2.18  # Required by PyGithub for decorators
-python-gitlab>=3.0.0
diff --git a/setup.py b/setup.py
@@ -6,6 +6,7 @@
     packages=find_packages(),
     install_requires=[
         "boto3",
+        "pydantic>=2.0",
         "requests"
     ],
     extras_require={

diff --git a/template_automation/app.py b/template_automation/app.py
@@ -10,6 +10,7 @@
 import json
 import logging
 import time
+import traceback
 from typing import Dict, Any, Optional
 from urllib.request import urlopen, Request, HTTPError
 
@@ -55,7 +56,7 @@ def to_template_settings(self) -> Dict[str, Any]:
         tags = {}
 
         # Get all model fields including extra ones (Pydantic v2)
-        all_fields = self.dict() if hasattr(self, 'dict') else dict(self)
+        all_fields = self.model_dump()
 
         for field_name, field_value in all_fields.items():
             if field_name not in exclude_fields:
@@ -70,7 +71,7 @@ def to_template_settings(self) -> Dict[str, Any]:
             "tags": tags
         }
 
-def log_api_call(method: str, url: str, headers: Dict = None, data: Any = None, response: requests.Response = None):
+def log_api_call(method: str, url: str, headers: Optional[Dict] = None, data: Any = None, response: Optional[requests.Response] = None):
     """Log detailed API call information for debugging."""
     logger.info(f"API Call: {method} {url}")
 
@@ -168,7 +169,6 @@ def get_provider():
             # For GitHub App tokens, it's better to use an endpoint that works with both user and app tokens
             test_url = f"{os.environ['GITHUB_API']}/repos/{os.environ['GITHUB_ORG_NAME']}"
             try:
-                import requests
                 response = requests.get(
                     test_url,
                     headers={'Authorization': f'Bearer {token}'},
@@ -208,7 +208,6 @@ def get_provider():
         except Exception as e:
             logger.error(f"Failed to initialize GitHub provider: {str(e)}")
             logger.error(f"Exception type: {type(e).__name__}")
-            import traceback
             logger.error(f"Full traceback: {traceback.format_exc()}")
             raise
 
@@ -240,7 +239,6 @@ def get_provider():
             logger.info("Testing GitLab API connectivity...")
             test_url = f"{os.environ['GITLAB_API']}/user"
             try:
-                import requests
                 response = requests.get(
                     test_url,
                     headers={'Private-Token': token},
@@ -261,7 +259,6 @@ def get_provider():
         except Exception as e:
             logger.error(f"Failed to initialize GitLab provider: {str(e)}")
             logger.error(f"Exception type: {type(e).__name__}")
-            import traceback
             logger.error(f"Full traceback: {traceback.format_exc()}")
             raise
     else:
@@ -353,11 +350,10 @@ def get_secret(secret_name: str) -> str:
         if hasattr(e, 'response'):
             logger.error(f"AWS error response: {e.response}")
 
-        import traceback
         logger.error(f"Full traceback: {traceback.format_exc()}")
         raise
 
-def send_cfn_response(event: dict, context, status: str, response_data: dict, physical_resource_id: str = None, reason: str = None):
+def send_cfn_response(event: dict, context, status: str, response_data: dict, physical_resource_id: Optional[str] = None, reason: Optional[str] = None):
     """Send response to CloudFormation for Custom Resource.
     
     Args:
@@ -520,7 +516,6 @@ def lambda_handler(event: dict, context) -> dict:
         except Exception as e:
             logger.error(f"[{request_id}] Repository operation failed: {str(e)}")
             logger.error(f"[{request_id}] Exception type: {type(e).__name__}")
-            import traceback
             logger.error(f"[{request_id}] Full traceback: {traceback.format_exc()}")
             raise
 
@@ -533,7 +528,6 @@ def lambda_handler(event: dict, context) -> dict:
             except Exception as e:
                 logger.error(f"[{request_id}] Failed to add team as admin: {str(e)}")
                 logger.error(f"[{request_id}] Exception type: {type(e).__name__}")
-                import traceback
                 logger.error(f"[{request_id}] Full traceback: {traceback.format_exc()}")
                 # Continue anyway, as team permissions are not critical
                 logger.info(f"[{request_id}] Continuing despite team permission error")
@@ -604,7 +598,6 @@ def lambda_handler(event: dict, context) -> dict:
         except Exception as e:
             logger.error(f"[{request_id}] Failed to clone template contents: {str(e)}")
             logger.error(f"[{request_id}] Exception type: {type(e).__name__}")
-            import traceback
             logger.error(f"[{request_id}] Full traceback: {traceback.format_exc()}")
             logger.info(f"[{request_id}] Continuing with repository setup despite cloning failure")
 
@@ -630,7 +623,6 @@ def lambda_handler(event: dict, context) -> dict:
         except Exception as e:
             logger.error(f"[{request_id}] Failed to write configuration file: {str(e)}")
             logger.error(f"[{request_id}] Exception type: {type(e).__name__}")
-            import traceback
             logger.error(f"[{request_id}] Full traceback: {traceback.format_exc()}")
             raise
 
@@ -650,7 +642,6 @@ def lambda_handler(event: dict, context) -> dict:
         except Exception as e:
             logger.error(f"[{request_id}] Failed to create merge/pull request: {str(e)}")
             logger.error(f"[{request_id}] Exception type: {type(e).__name__}")
-            import traceback
             logger.error(f"[{request_id}] Full traceback: {traceback.format_exc()}")
             raise
 
@@ -692,7 +683,6 @@ def lambda_handler(event: dict, context) -> dict:
     except Exception as e:
         logger.error(f"[{request_id}] Lambda function failed with error: {str(e)}")
         logger.error(f"[{request_id}] Exception type: {type(e).__name__}")
-        import traceback
         logger.error(f"[{request_id}] Full traceback: {traceback.format_exc()}")
 
         # Send failure response to CloudFormation

diff --git a/template_automation/github_provider.py b/template_automation/github_provider.py
@@ -65,7 +65,7 @@ def __init__(
             import urllib3
             urllib3.disable_warnings(urllib3.exceptions.InsecureRequestWarning)
 
-    def _request(self, method: str, url: str, **kwargs) -> Dict[str, Any]:
+    def _request(self, method: str, url: str, **kwargs) -> Any:
         """Make a request to the GitHub API.
         
         Args:
@@ -74,7 +74,7 @@ def _request(self, method: str, url: str, **kwargs) -> Dict[str, Any]:
             **kwargs: Additional arguments to pass to requests
             
         Returns:
-            Response data as dictionary
+            Response data (dict or list depending on the endpoint)
             
         Raises:
             requests.exceptions.RequestException: On request failure
@@ -262,19 +262,6 @@ def get_repository(
 
                         # Retry with the original error
                         raise org_err
-
-                    # Set topics if provided
-                    if settings.topics:
-                        try:
-                            logger.info(f"Setting repository topics: {settings.topics}")
-                            self._request('PUT', f'/repos/{self.organization}/{name}/topics',
-                                        json={'names': settings.topics})
-                        except Exception as topic_err:
-                            logger.warning(f"Failed to set repository topics: {str(topic_err)}")
-                            logger.warning("Continuing without topics")
-
-                    logger.info(f"Successfully created repository {name}")
-                    return repo
                 except Exception as create_err:
                     logger.error(f"Failed to create repository {name}: {str(create_err)}")
                     if hasattr(create_err, 'response') and create_err.response is not None:
@@ -380,15 +367,17 @@ def write_file(
                      e.response.status_code == 404)
 
             if is_404:
-                method = 'PUT' if branch != 'main' else 'POST'
+                # GitHub Contents API always uses PUT for creating/updating files
+                method = 'PUT'
                 data = {}
             else:
                 # Re-raise other errors
                 raise
 
         # Prepare commit data
+        is_update = 'sha' in data
         data.update({
-            'message': message or f"{'Update' if method == 'PUT' else 'Create'} {file.path}",
+            'message': message or f"{'Update' if is_update else 'Create'} {file.path}",
             'branch': branch
         })
 
@@ -598,8 +587,12 @@ def get_repository_url(self, repo_name: str) -> str:
         Returns:
             Web interface URL for the repository
         """
-        # For GitHub Enterprise, we need a different URL format than API URLs
-        return f"{self.api_base_url}/{self.organization}/{repo_name}"
+        # For GitHub Enterprise, the API base URL may contain /api/v3 which
+        # should not be part of the web URL. Strip it to get the web base URL.
+        web_base = self.api_base_url.rstrip('/')
+        if web_base.endswith('/api/v3'):
+            web_base = web_base[:-len('/api/v3')]
+        return f"{web_base}/{self.organization}/{repo_name}"
 
     def set_team_permission(
         self,

diff --git a/template_automation/gitlab_provider.py b/template_automation/gitlab_provider.py
@@ -58,7 +58,7 @@ def group_id(self) -> int:
             self._group_id = group['id']
         return self._group_id
 
-    def _request(self, method: str, url: str, **kwargs) -> Dict[str, Any]:
+    def _request(self, method: str, url: str, **kwargs) -> Any:
         """Make a request to the GitLab API.
         
         Args:
@@ -67,7 +67,7 @@ def _request(self, method: str, url: str, **kwargs) -> Dict[str, Any]:
             **kwargs: Additional arguments to pass to requests
             
         Returns:
-            Response data as dictionary
+            Response data (dict or list depending on the endpoint)
             
         Raises:
             requests.exceptions.RequestException: On request failure
@@ -129,8 +129,11 @@ def get_repository(
         try:
             path = urllib.parse.quote(f"{self.organization}/{name}", safe="")
             return self._request('GET', f'/api/v4/projects/{path}')
-        except requests.exceptions.HTTPError as e:
-            if e.response.status_code == 404 and create:
+        except requests.exceptions.RequestException as e:
+            is_404 = (hasattr(e, 'response') and 
+                      e.response is not None and 
+                      e.response.status_code == 404)
+            if is_404 and create:
                 if not settings:
                     settings = RepositorySettings()
 
@@ -156,7 +159,9 @@ def get_repository(
                 # Wait for project to be ready
                 time.sleep(2)
                 return self._request('GET', f'/api/v4/projects/{project["id"]}')
-            raise ValueError(f"Repository {name} not found")
+            if is_404:
+                raise ValueError(f"Repository {name} not found")
+            raise
 
     def get_branch(self, repo_name: str, branch: str) -> Dict[str, Any]:
         """Get a branch from a repository.

diff --git a/template_automation/models.py b/template_automation/models.py
@@ -195,13 +195,8 @@ class TemplateConfig(BaseModel):
     )
     workflows: List[WorkflowConfig] = Field(default_factory=list)
 
-    class Config:
-        """Pydantic model configuration.
-        
-        This inner class defines metadata for the TemplateConfig model,
-        including example configurations and schema information.
-        """
-        json_schema_extra = {
+    model_config = {
+        "json_schema_extra": {
             "example": {
                 "pr": {
                     "title_template": "Initialize {{ repo_name }} from template",
@@ -215,3 +210,4 @@ class Config:
                 "workflows": []
             }
         }
+    }
diff --git a/template_automation/repository_provider.py b/template_automation/repository_provider.py
@@ -19,8 +19,7 @@ class FileContent(BaseModel):
     content: Any = Field(..., description="Content of the file (string or bytes)")
     encoding: str = Field(default="utf-8", description="Encoding of the content")
 
-    class Config:
-        arbitrary_types_allowed = True
+    model_config = {"arbitrary_types_allowed": True}
 
 class MergeRequestSettings(BaseModel):
     """Settings for merge/pull request creation."""

diff --git a/tests/__init__.py b/tests/__init__.py
@@ -0,0 +1 @@
+# Test package initialization
diff --git a/tests/integration/__init__.py b/tests/integration/__init__.py
@@ -0,0 +1 @@
+# Integration test package initialization