From b2d952cafacde80684f387d3a033a73435b84478 Mon Sep 17 00:00:00 2001 From: Ori Kotek Date: Tue, 11 Jul 2023 16:55:09 +0300 Subject: [PATCH] 1. Move deployment_type to configuration.toml 2. Lint 3. Inject GitHub app installation ID into GitHub provider using the settings mechanism. --- pr_agent/agent/pr_agent.py | 1 - pr_agent/algo/language_handler.py | 2 +- pr_agent/git_providers/git_provider.py | 13 ++++++++++--- pr_agent/git_providers/github_provider.py | 1 + pr_agent/git_providers/gitlab_provider.py | 22 ++++++++++++++-------- pr_agent/servers/github_app.py | 7 ++++--- pr_agent/servers/github_polling.py | 3 ++- pr_agent/servers/gitlab_polling.py | 5 ++--- pr_agent/settings/.secrets_template.toml | 5 ----- pr_agent/settings/configuration.toml | 11 +++++++---- pr_agent/tools/pr_questions.py | 1 - pr_agent/tools/pr_reviewer.py | 1 - requirements.txt | 2 ++ tests/unit/test_convert_to_markdown.py | 2 +- tests/unit/test_language_handler.py | 12 +++++++----- tests/unit/test_parse_code_suggestion.py | 4 ++-- 16 files changed, 53 insertions(+), 39 deletions(-) diff --git a/pr_agent/agent/pr_agent.py b/pr_agent/agent/pr_agent.py index 7e4a6c22..44815f56 100644 --- a/pr_agent/agent/pr_agent.py +++ b/pr_agent/agent/pr_agent.py @@ -1,5 +1,4 @@ import re -from typing import Optional from pr_agent.tools.pr_questions import PRQuestions from pr_agent.tools.pr_reviewer import PRReviewer diff --git a/pr_agent/algo/language_handler.py b/pr_agent/algo/language_handler.py index efc038ca..db99d20a 100644 --- a/pr_agent/algo/language_handler.py +++ b/pr_agent/algo/language_handler.py @@ -93,7 +93,7 @@ def sort_files_by_main_languages(languages: Dict, files: list): for ext in main_extensions: main_extensions_flat.extend(ext) - for extensions, lang in zip(main_extensions, languages_sorted_list): + for extensions, lang in zip(main_extensions, languages_sorted_list): # noqa: B905 tmp = [] for file in files_filtered: extension_str = f".{file.filename.split('.')[-1]}" diff --git a/pr_agent/git_providers/git_provider.py b/pr_agent/git_providers/git_provider.py index b5d8df23..a30df90b 100644 --- a/pr_agent/git_providers/git_provider.py +++ b/pr_agent/git_providers/git_provider.py @@ -1,5 +1,4 @@ - -from abc import ABC +from abc import ABC, abstractmethod from dataclasses import dataclass @@ -13,27 +12,35 @@ class FilePatchInfo: class GitProvider(ABC): + @abstractmethod def get_diff_files(self) -> list[FilePatchInfo]: pass + @abstractmethod def publish_comment(self, pr_comment: str, is_temporary: bool = False): pass + @abstractmethod def remove_initial_comment(self): pass + @abstractmethod def get_languages(self): pass + @abstractmethod def get_pr_branch(self): pass + @abstractmethod def get_user_id(self): pass + @abstractmethod def get_pr_description(self): pass + def get_main_pr_language(languages, files) -> str: """ Get the main language of the commit. Return an empty string if cannot determine. @@ -72,4 +79,4 @@ def get_main_pr_language(languages, files) -> str: except Exception: pass - return main_language_str \ No newline at end of file + return main_language_str diff --git a/pr_agent/git_providers/github_provider.py b/pr_agent/git_providers/github_provider.py index 0b65c539..ecc624c7 100644 --- a/pr_agent/git_providers/github_provider.py +++ b/pr_agent/git_providers/github_provider.py @@ -6,6 +6,7 @@ from urllib.parse import urlparse from github import AppAuthentication, Github from pr_agent.config_loader import settings + from .git_provider import FilePatchInfo diff --git a/pr_agent/git_providers/gitlab_provider.py b/pr_agent/git_providers/gitlab_provider.py index 2082fa33..e9279a82 100644 --- a/pr_agent/git_providers/gitlab_provider.py +++ b/pr_agent/git_providers/gitlab_provider.py @@ -1,6 +1,8 @@ -from urllib.parse import urlparse -import gitlab +import logging from typing import Optional, Tuple +from urllib.parse import urlparse + +import gitlab from pr_agent.config_loader import settings @@ -9,24 +11,28 @@ from .git_provider import FilePatchInfo, GitProvider class GitLabProvider(GitProvider): def __init__(self, merge_request_url: Optional[str] = None): + gitlab_url = settings.get("GITLAB.URL", None) + if not gitlab_url: + raise ValueError("GitLab URL is not set in the config file") + gitlab_access_token = settings.get("GITLAB.PERSONAL_ACCESS_TOKEN", None) + if not gitlab_access_token: + raise ValueError("GitLab personal access token is not set in the config file") self.gl = gitlab.Gitlab( - settings.get("GITLAB.URL"), - private_token=settings.get("GITLAB.PERSONAL_ACCESS_TOKEN") + gitlab_url, + gitlab_access_token ) - self.id_project = None self.id_mr = None self.mr = None self.temp_comments = [] - - self.set_merge_request(merge_request_url) + self._set_merge_request(merge_request_url) @property def pr(self): '''The GitLab terminology is merge request (MR) instead of pull request (PR)''' return self.mr - def set_merge_request(self, merge_request_url: str): + def _set_merge_request(self, merge_request_url: str): self.id_project, self.id_mr = self._parse_merge_request_url(merge_request_url) self.mr = self._get_merge_request() diff --git a/pr_agent/servers/github_app.py b/pr_agent/servers/github_app.py index 6dc5782b..52425651 100644 --- a/pr_agent/servers/github_app.py +++ b/pr_agent/servers/github_app.py @@ -35,7 +35,8 @@ async def handle_github_webhooks(request: Request, response: Response): async def handle_request(body): action = body.get("action", None) installation_id = body.get("installation", {}).get("id", None) - agent = PRAgent(installation_id) + settings.set("GITHUB.INSTALLATION_ID", installation_id) + agent = PRAgent() if action == 'created': if "comment" not in body: return {} @@ -66,8 +67,8 @@ async def root(): def start(): - if settings.get("GITHUB.DEPLOYMENT_TYPE", "user") != "app": - raise Exception("Please set deployment type to app in .secrets.toml file") + # Override the deployment type to app + settings.set("GITHUB.DEPLOYMENT_TYPE", "app") app = FastAPI() app.include_router(router) diff --git a/pr_agent/servers/github_polling.py b/pr_agent/servers/github_polling.py index 06293fd6..e8cc4223 100644 --- a/pr_agent/servers/github_polling.py +++ b/pr_agent/servers/github_polling.py @@ -76,7 +76,8 @@ async def polling_loop(): if comment['user']['login'] == user_id: continue comment_body = comment['body'] if 'body' in comment else '' - commenter_github_user = comment['user']['login'] if 'user' in comment else '' + commenter_github_user = comment['user']['login'] \ + if 'user' in comment else '' logging.info(f"Commenter: {commenter_github_user}\nComment: {comment_body}") user_tag = "@" + user_id if user_tag not in comment_body: diff --git a/pr_agent/servers/gitlab_polling.py b/pr_agent/servers/gitlab_polling.py index 4409d176..c240310e 100644 --- a/pr_agent/servers/gitlab_polling.py +++ b/pr_agent/servers/gitlab_polling.py @@ -1,12 +1,11 @@ import asyncio import time -from urllib.parse import urlparse + import gitlab + from pr_agent.agent.pr_agent import PRAgent - from pr_agent.config_loader import settings - gl = gitlab.Gitlab( settings.get("GITLAB.URL"), private_token=settings.get("GITLAB.PERSONAL_ACCESS_TOKEN") diff --git a/pr_agent/settings/.secrets_template.toml b/pr_agent/settings/.secrets_template.toml index 7ce3d52f..eb2f9b76 100644 --- a/pr_agent/settings/.secrets_template.toml +++ b/pr_agent/settings/.secrets_template.toml @@ -11,9 +11,6 @@ key = "" # Acquire through https://platform.openai.com org = "" # Optional, may be commented out. [github] -# The type of deployment to create. Valid values are 'app' or 'user'. -deployment_type = "user" - # ---- Set the following only for deployment type == "user" user_token = "" # A GitHub personal access token with 'repo' scope. @@ -30,5 +27,3 @@ webhook_secret = "" # Optional, may be commented out. # Gitlab personal access token personal_access_token = "" -# URL to the gitlab service -gitlab_url = "https://gitlab.com" diff --git a/pr_agent/settings/configuration.toml b/pr_agent/settings/configuration.toml index 6bb6db97..f0a646d1 100644 --- a/pr_agent/settings/configuration.toml +++ b/pr_agent/settings/configuration.toml @@ -11,18 +11,21 @@ require_security_review=true extended_code_suggestions=false num_code_suggestions=4 - [pr_questions] +[github] +# The type of deployment to create. Valid values are 'app' or 'user'. +deployment_type = "user" + [gitlab] # URL to the gitlab service gitlab_url = "https://gitlab.com" -# Polling (either proheheject id or namespace/project_name) syntax can be used -projects_to_monitor = ['nuclai/algo', 'nuclai/pr-agent-test'] +# Polling (either project id or namespace/project_name) syntax can be used +projects_to_monitor = ['org_name/repo_name'] # Polling trigger magic_word = "AutoReview" # Polling interval -polling_interval_seconds = 300 \ No newline at end of file +polling_interval_seconds = 30 diff --git a/pr_agent/tools/pr_questions.py b/pr_agent/tools/pr_questions.py index eb8e6bcb..1c26cf99 100644 --- a/pr_agent/tools/pr_questions.py +++ b/pr_agent/tools/pr_questions.py @@ -1,6 +1,5 @@ import copy import logging -from typing import Optional from jinja2 import Environment, StrictUndefined diff --git a/pr_agent/tools/pr_reviewer.py b/pr_agent/tools/pr_reviewer.py index f0a8c485..504548b1 100644 --- a/pr_agent/tools/pr_reviewer.py +++ b/pr_agent/tools/pr_reviewer.py @@ -1,7 +1,6 @@ import copy import json import logging -from typing import Optional from jinja2 import Environment, StrictUndefined diff --git a/requirements.txt b/requirements.txt index 4a6a6255..64134909 100644 --- a/requirements.txt +++ b/requirements.txt @@ -7,3 +7,5 @@ Jinja2==3.1.2 tiktoken==0.4.0 uvicorn==0.22.0 python-gitlab==3.15.0 +pytest~=7.4.0 +aiohttp~=3.8.4 diff --git a/tests/unit/test_convert_to_markdown.py b/tests/unit/test_convert_to_markdown.py index 08a49f76..bfd9c9b5 100644 --- a/tests/unit/test_convert_to_markdown.py +++ b/tests/unit/test_convert_to_markdown.py @@ -1,6 +1,6 @@ # Generated by CodiumAI from pr_agent.algo.utils import convert_to_markdown -import pytest + """ Code Analysis diff --git a/tests/unit/test_language_handler.py b/tests/unit/test_language_handler.py index 5807b09b..875ec1a7 100644 --- a/tests/unit/test_language_handler.py +++ b/tests/unit/test_language_handler.py @@ -1,15 +1,15 @@ # Generated by CodiumAI + from pr_agent.algo.language_handler import sort_files_by_main_languages - -import pytest - """ Code Analysis Objective: -The objective of the function is to sort a list of files by their main language, putting the files that are in the main language first and the rest of the files after. It takes in a dictionary of languages and their sizes, and a list of files. +The objective of the function is to sort a list of files by their main language, putting the files that are in the main +language first and the rest of the files after. It takes in a dictionary of languages and their sizes, and a list of +files. Inputs: - languages: a dictionary containing the languages and their sizes @@ -33,6 +33,8 @@ Additional aspects: - The function uses the filter_bad_extensions function to filter out files with bad extensions - The function uses a rest_files dictionary to store the files that do not belong to any of the main extensions """ + + class TestSortFilesByMainLanguages: # Tests that files are sorted by main language, with files in main language first and the rest after def test_happy_path_sort_files_by_main_languages(self): @@ -118,4 +120,4 @@ class TestSortFilesByMainLanguages: {'language': 'C++', 'files': [files[2], files[7]]}, {'language': 'Other', 'files': []} ] - assert sort_files_by_main_languages(languages, files) == expected_output \ No newline at end of file + assert sort_files_by_main_languages(languages, files) == expected_output diff --git a/tests/unit/test_parse_code_suggestion.py b/tests/unit/test_parse_code_suggestion.py index 082fed77..87e3cac8 100644 --- a/tests/unit/test_parse_code_suggestion.py +++ b/tests/unit/test_parse_code_suggestion.py @@ -70,7 +70,7 @@ class TestParseCodeSuggestion: 'before': 'Before 1', 'after': 'After 1' } - expected_output = " **suggestion:** Suggestion 1\n **description:** Description 1\n **before:** Before 1\n **after:** After 1\n\n" + expected_output = " **suggestion:** Suggestion 1\n **description:** Description 1\n **before:** Before 1\n **after:** After 1\n\n" # noqa: E501 assert parse_code_suggestion(code_suggestions) == expected_output # Tests that function returns correct output when input dictionary has 'code example' key @@ -84,5 +84,5 @@ class TestParseCodeSuggestion: 'after': 'After 2' } } - expected_output = " **suggestion:** Suggestion 2\n **description:** Description 2\n - **code example:**\n - **before:**\n ```\n Before 2\n ```\n - **after:**\n ```\n After 2\n ```\n\n" + expected_output = " **suggestion:** Suggestion 2\n **description:** Description 2\n - **code example:**\n - **before:**\n ```\n Before 2\n ```\n - **after:**\n ```\n After 2\n ```\n\n" # noqa: E501 assert parse_code_suggestion(code_suggestions) == expected_output