From 45e2604496614cfea7e8284f586eb1c7146cca95 Mon Sep 17 00:00:00 2001 From: Christopher Dignam Date: Thu, 14 Apr 2022 11:35:08 -0400 Subject: [PATCH 1/6] improve http error handling We should retry all http errors when merging PRs. We should also report to Sentry for any weird behavior. --- bot/kodiak/queries/__init__.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/bot/kodiak/queries/__init__.py b/bot/kodiak/queries/__init__.py index 9e45067c1..a2f04707d 100644 --- a/bot/kodiak/queries/__init__.py +++ b/bot/kodiak/queries/__init__.py @@ -840,7 +840,7 @@ async def send_query( query: str, variables: Mapping[str, Union[str, int, None]], installation_id: str, - ) -> Optional[GraphQLResponse]: + ) -> GraphQLResponse | http.HTTPError: log = self.log token = await get_token_for_install( @@ -857,9 +857,9 @@ async def send_query( log = log.bind(rate_limit=rate_limit) try: res.raise_for_status() - except http.HTTPError: + except http.HTTPError as e: log.warning("github api request error", res=res, exc_info=True) - return None + return e return cast(GraphQLResponse, res.json()) async def get_api_features(self) -> ApiFeatures | None: @@ -990,7 +990,7 @@ def get_file_expression() -> str: file_expression=get_file_expression(), ) - async def get_event_info(self, pr_number: int) -> Optional[EventInfoResponse]: + async def get_event_info(self, pr_number: int) -> EventInfoResponse | Literal["no_repository", "no_pr", "empty_response", "pr_parse_failure", "no_config"]: """ Retrieve all the information we need to evaluate a pull request From deeca3294bb4cb28cef5daa6242f3a36d563f467 Mon Sep 17 00:00:00 2001 From: Christopher Dignam Date: Thu, 14 Apr 2022 13:20:01 -0400 Subject: [PATCH 2/6] wip --- bot/kodiak/pull_request.py | 11 ++++++++--- bot/kodiak/queries/__init__.py | 4 +++- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/bot/kodiak/pull_request.py b/bot/kodiak/pull_request.py index 866bc2b3c..9bb4cfd6c 100644 --- a/bot/kodiak/pull_request.py +++ b/bot/kodiak/pull_request.py @@ -90,10 +90,15 @@ async def evaluate_pr( ), timeout=60, ) - if pr is None: - log.info("failed to get_pr") - return try: + if pr is None: + if merging: + raise ApiCallException( + method="kodiak/get_pull_request", + http_status_code=0, + response=b"", + ) + return None await asyncio.wait_for( mergeable( api=pr, diff --git a/bot/kodiak/queries/__init__.py b/bot/kodiak/queries/__init__.py index a2f04707d..ac580f012 100644 --- a/bot/kodiak/queries/__init__.py +++ b/bot/kodiak/queries/__init__.py @@ -968,6 +968,7 @@ async def get_config_for_ref( parsed_config = parse_config(data) if parsed_config is None: + self.log.info("no config found") return None def get_file_expression() -> str: @@ -990,7 +991,7 @@ def get_file_expression() -> str: file_expression=get_file_expression(), ) - async def get_event_info(self, pr_number: int) -> EventInfoResponse | Literal["no_repository", "no_pr", "empty_response", "pr_parse_failure", "no_config"]: + async def get_event_info(self, pr_number: int) -> EventInfoResponse: """ Retrieve all the information we need to evaluate a pull request @@ -1011,6 +1012,7 @@ async def get_event_info(self, pr_number: int) -> EventInfoResponse | Literal["n installation_id=self.installation_id, ) if res is None: + log.warning("empty API response") return None data = res.get("data") From e3081659c6ae630ba26fff841d12e051162fef60 Mon Sep 17 00:00:00 2001 From: Christopher Dignam Date: Thu, 14 Apr 2022 13:30:20 -0400 Subject: [PATCH 3/6] wip --- bot/kodiak/logging.py | 2 +- bot/kodiak/queries/__init__.py | 7 +++++-- bot/kodiak/test_logging.py | 3 ++- 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/bot/kodiak/logging.py b/bot/kodiak/logging.py index 3d9cc3897..ac4d317ae 100644 --- a/bot/kodiak/logging.py +++ b/bot/kodiak/logging.py @@ -4,7 +4,7 @@ import sentry_sdk import structlog -from requests import Response +from httpx import Response from sentry_sdk import capture_event from sentry_sdk.integrations.logging import LoggingIntegration from sentry_sdk.utils import event_from_exception diff --git a/bot/kodiak/queries/__init__.py b/bot/kodiak/queries/__init__.py index ac580f012..9f7467e08 100644 --- a/bot/kodiak/queries/__init__.py +++ b/bot/kodiak/queries/__init__.py @@ -935,6 +935,7 @@ def get_bot_reviews(self, *, reviews: List[PRReviewSchema]) -> List[PRReview]: async def get_config_for_ref( self, *, ref: str, org_repo_default_branch: str | None ) -> CfgInfo | None: + log = self.log.bind(ref=ref, org_repo_default_branch=org_repo_default_branch) repo_root_config_expression = create_root_config_file_expression(branch=ref) repo_github_config_expression = create_github_config_file_expression(branch=ref) org_root_config_expression: str | None = None @@ -959,16 +960,18 @@ async def get_config_for_ref( ), installation_id=self.installation_id, ) + log = log.bind(res=res) if res is None: + log.info("get_config api error") return None data = res.get("data") if data is None: - self.log.error("could not fetch default branch name", res=res) + log.error("could not fetch default branch name") return None parsed_config = parse_config(data) if parsed_config is None: - self.log.info("no config found") + log.info("no config in response") return None def get_file_expression() -> str: diff --git a/bot/kodiak/test_logging.py b/bot/kodiak/test_logging.py index 929be9f09..e797341a0 100644 --- a/bot/kodiak/test_logging.py +++ b/bot/kodiak/test_logging.py @@ -3,7 +3,8 @@ from typing import Any, cast import pytest -from requests import PreparedRequest, Request, Response +from httpx import Request, Response +from requests import PreparedRequest from kodiak.logging import ( SentryLevel, From d2a932addde19424f2592ad763e8adff1c8e94fc Mon Sep 17 00:00:00 2001 From: Christopher Dignam Date: Thu, 14 Apr 2022 13:33:58 -0400 Subject: [PATCH 4/6] fix? --- bot/kodiak/test_logging.py | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/bot/kodiak/test_logging.py b/bot/kodiak/test_logging.py index e797341a0..1c265bd9b 100644 --- a/bot/kodiak/test_logging.py +++ b/bot/kodiak/test_logging.py @@ -4,7 +4,6 @@ import pytest from httpx import Request, Response -from requests import PreparedRequest from kodiak.logging import ( SentryLevel, @@ -161,14 +160,12 @@ def test_add_request_info_processor() -> None: url = "https://api.example.com/v1/me" payload = dict(user_id=54321) req = Request("POST", url, json=payload) - res = Response() - res.status_code = 500 - res.url = url - res.reason = "Internal Server Error" - cast( - Any, res - )._content = b"Your request could not be completed due to an internal error." - res.request = cast(PreparedRequest, req.prepare()) # type: ignore + res = Response( + status_code=500, + content=b"Your request could not be completed due to an internal error.", + request=req, + ) + event_dict = add_request_info_processor( None, None, dict(event="request failed", res=res) ) From 9ab4e888d9bc34e3cd27eab28f6dfad7bb5acb92 Mon Sep 17 00:00:00 2001 From: Christopher Dignam Date: Thu, 14 Apr 2022 13:34:00 -0400 Subject: [PATCH 5/6] . --- kodiak.code-workspace | 3 +++ 1 file changed, 3 insertions(+) diff --git a/kodiak.code-workspace b/kodiak.code-workspace index f17b9f219..20672e52c 100644 --- a/kodiak.code-workspace +++ b/kodiak.code-workspace @@ -14,6 +14,9 @@ }, { "path": "infrastructure" + }, + { + "path": "." } ], "settings": {} From 0543df6b1e778f7ba1e37b93bc902bc61b59bef3 Mon Sep 17 00:00:00 2001 From: Christopher Dignam Date: Thu, 14 Apr 2022 13:36:07 -0400 Subject: [PATCH 6/6] ban imports --- bot/poetry.lock | 18 +++++++++++++++++- bot/pyproject.toml | 1 + bot/tox.ini | 3 +++ 3 files changed, 21 insertions(+), 1 deletion(-) diff --git a/bot/poetry.lock b/bot/poetry.lock index aaa668c3d..66913a90a 100644 --- a/bot/poetry.lock +++ b/bot/poetry.lock @@ -268,6 +268,18 @@ attrs = "*" flake8 = ">=3.2.1" pyflakes = ">=2.1.1" +[[package]] +name = "flake8-tidy-imports" +version = "4.6.0" +description = "A flake8 plugin that helps you write tidier imports." +category = "dev" +optional = false +python-versions = ">=3.7" + +[package.dependencies] +flake8 = ">=3.8.0" +importlib-metadata = {version = "*", markers = "python_version < \"3.8\""} + [[package]] name = "greenlet" version = "1.1.1" @@ -1047,7 +1059,7 @@ python-versions = "*" [metadata] lock-version = "1.1" python-versions = "^3.7" -content-hash = "c46e679a8a20316d87bf883d450d68d978addbc6cb1bd29507e79bdf89407001" +content-hash = "55d80a5993cd4196097f2d246ecaadb0f47812e67ac68ec6e934ae396f5f5a5f" [metadata.files] appnope = [ @@ -1243,6 +1255,10 @@ flake8-pyi = [ {file = "flake8-pyi-20.10.0.tar.gz", hash = "sha256:cee3b20a5123152c697870e7e800b60e3c95eb89e272a2b63d8cf55cafb0472c"}, {file = "flake8_pyi-20.10.0-py2.py3-none-any.whl", hash = "sha256:ff5dfc40bffa878f6ce95bcfd9a6ad14c44b85cbe99c4864e729301bf54267f0"}, ] +flake8-tidy-imports = [ + {file = "flake8-tidy-imports-4.6.0.tar.gz", hash = "sha256:3e193d8c4bb4492408a90e956d888b27eed14c698387c9b38230da3dad78058f"}, + {file = "flake8_tidy_imports-4.6.0-py3-none-any.whl", hash = "sha256:6ae9f55d628156e19d19f4c359dd5d3e95431a9bd514f5e2748c53c1398c66b2"}, +] greenlet = [ {file = "greenlet-1.1.1-cp27-cp27m-macosx_10_14_x86_64.whl", hash = "sha256:476ba9435afaead4382fbab8f1882f75e3fb2285c35c9285abb3dd30237f9142"}, {file = "greenlet-1.1.1-cp27-cp27m-manylinux1_x86_64.whl", hash = "sha256:44556302c0ab376e37939fd0058e1f0db2e769580d340fb03b01678d1ff25f68"}, diff --git a/bot/pyproject.toml b/bot/pyproject.toml index f9295b966..eeff3200e 100644 --- a/bot/pyproject.toml +++ b/bot/pyproject.toml @@ -42,6 +42,7 @@ flake8-pie = "0.7.1" isort = "^4.3" pytest-cov = "^2.10" flake8-pyi = "^20.10" +flake8-tidy-imports = "^4.6.0" [tool.poetry.plugins."pytest11"] "pytest_plugin" = "pytest_plugin.plugin" diff --git a/bot/tox.ini b/bot/tox.ini index 76a155427..4b275dc51 100644 --- a/bot/tox.ini +++ b/bot/tox.ini @@ -9,6 +9,9 @@ filterwarnings = ignore::pytest.PytestDeprecationWarning ignore::DeprecationWarning:asyncio_redis [flake8] +banned-modules = + requests = Use httpx instead., + ban-relative-imports = true ignore = ; formatting handled by black ; https://pycodestyle.readthedocs.io/en/latest/intro.html#error-codes