Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: better handle CancelledError from static analysis #275

Merged
merged 2 commits into from
Oct 5, 2023
Merged

chore: better handle CancelledError from static analysis #275

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1da13a0
chore: handle internal `send_single_upload_put` exceptions
giovanni-guidini Oct 4, 2023
0fed329
chore: better handle CancelledError from static analysis
giovanni-guidini Oct 4, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 35 additions & 19 deletions codecov_cli/services/staticanalysis/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,14 @@
for el in files_that_need_upload:
all_tasks.append(send_single_upload_put(client, all_data, el))
bar.update(1, all_data[el["filepath"]])
resps = await asyncio.gather(*all_tasks)
try:
resps = await asyncio.gather(*all_tasks)
except asyncio.CancelledError:
message = (
"Unknown error cancelled the upload tasks.\n"
+ f"Uploaded {len(uploaded_files)}/{len(files_that_need_upload)} files successfully."
)
raise click.ClickException(message)
for resp in resps:
if resp["succeeded"]:
uploaded_files.append(resp["filepath"])
Expand Down Expand Up @@ -188,35 +195,44 @@
)


async def send_single_upload_put(client, all_data, el):
async def send_single_upload_put(client, all_data, el) -> typing.Dict:
retryable_statuses = (429,)
presigned_put = el["raw_upload_location"]
number_retries = 5
for current_retry in range(number_retries):
response = await client.put(
presigned_put, data=json.dumps(all_data[el["filepath"]])
)
if response.status_code < 300:
return {
"status_code": response.status_code,
"filepath": el["filepath"],
"succeeded": True,
}
if response.status_code in retryable_statuses:
await asyncio.sleep(2**current_retry)
try:
for current_retry in range(number_retries):
response = await client.put(
presigned_put, data=json.dumps(all_data[el["filepath"]])
)
if response.status_code < 300:
return {
"status_code": response.status_code,
"filepath": el["filepath"],
"succeeded": True,
}
if response.status_code in retryable_statuses:
await asyncio.sleep(2**current_retry)

Check warning on line 214 in codecov_cli/services/staticanalysis/__init__.py

View check run for this annotation

Codecov / codecov/patch

codecov_cli/services/staticanalysis/__init__.py#L214 

Added line #L214 was not covered by tests
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It'd be good to write a test that covers this line but otherwise lgtm

status_code = response.status_code
message_to_warn = response.text
exception = None
except httpx.HTTPError as exp:
status_code = None
exception = type(exp)
message_to_warn = str(exp)
logger.warning(
"Unable to send data",
"Unable to send single_upload_put",
extra=dict(
extra_log_attributes=dict(
response=response.text,
message=message_to_warn,
exception=exception,
filepath=el["filepath"],
url=presigned_put,
latest_status_code=response.status_code,
latest_status_code=status_code,
)
),
)
return {
"status_code": response.status_code,
"status_code": status_code,
"exception": exception,
"filepath": el["filepath"],
"succeeded": False,
}
Expand Down
172 changes: 170 additions & 2 deletions tests/services/static_analysis/test_static_analysis_service.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,19 @@
from asyncio import CancelledError
from pathlib import Path
from unittest.mock import MagicMock

import click
import httpx
import pytest
import requests
import responses
from responses import matchers

from codecov_cli.services.staticanalysis import process_files, run_analysis_entrypoint
from codecov_cli.services.staticanalysis import (
process_files,
run_analysis_entrypoint,
send_single_upload_put,
)
from codecov_cli.services.staticanalysis.types import (
FileAnalysisRequest,
FileAnalysisResult,
Expand Down Expand Up @@ -67,7 +73,7 @@ def imap_side_effect(mapped_func, files):
)

@pytest.mark.asyncio
async def test_static_analysis_service(self, mocker):
async def test_static_analysis_service_success(self, mocker):
mock_file_finder = mocker.patch(
"codecov_cli.services.staticanalysis.select_file_finder"
)
Expand Down Expand Up @@ -143,6 +149,168 @@ async def side_effect(*args, **kwargs):
"raw_upload_location": "http://storage-url",
}

@pytest.mark.asyncio
async def test_static_analysis_service_CancelledError(self, mocker):
mock_file_finder = mocker.patch(
"codecov_cli.services.staticanalysis.select_file_finder"
)
mock_send_upload_put = mocker.patch(
"codecov_cli.services.staticanalysis.send_single_upload_put"
)

async def side_effect(client, all_data, el):
if el["filepath"] == "samples/inputs/sample_001.py":
return {
"status_code": 204,
"filepath": el["filepath"],
"succeeded": True,
}
raise CancelledError("Pretending something cancelled this task")

mock_send_upload_put.side_effect = side_effect

files_found = map(
lambda filename: FileAnalysisRequest(str(filename), Path(filename)),
[
"samples/inputs/sample_001.py",
"samples/inputs/sample_002.py",
],
)
mock_file_finder.return_value.find_files = MagicMock(return_value=files_found)
with responses.RequestsMock() as rsps:
rsps.add(
responses.POST,
"https://api.codecov.io/staticanalysis/analyses",
json={
"external_id": "externalid",
"filepaths": [
{
"state": "created",
"filepath": "samples/inputs/sample_001.py",
"raw_upload_location": "http://storage-url-001",
},
{
"state": "created",
"filepath": "samples/inputs/sample_002.py",
"raw_upload_location": "http://storage-url-002",
},
],
},
status=200,
match=[
matchers.header_matcher({"Authorization": "Repotoken STATIC_TOKEN"})
],
)

with pytest.raises(click.ClickException) as exp:
await run_analysis_entrypoint(
config={},
folder=".",
numberprocesses=1,
pattern="*.py",
token="STATIC_TOKEN",
commit="COMMIT",
should_force=False,
folders_to_exclude=[],
enterprise_url=None,
)
assert "Unknown error cancelled the upload tasks." in str(exp.value)
mock_file_finder.assert_called_with({})
mock_file_finder.return_value.find_files.assert_called()
assert mock_send_upload_put.call_count == 2

@pytest.mark.asyncio
async def test_send_single_upload_put_success(self, mocker):
mock_client = MagicMock()

async def side_effect(presigned_put, data):
if presigned_put == "http://storage-url-001":
return httpx.Response(status_code=204)

mock_client.put.side_effect = side_effect

all_data = {
"file-001": {"some": "data", "id": "1"},
"file-002": {"some": "data", "id": "2"},
"file-003": {"some": "data", "id": "3"},
}

success_response = await send_single_upload_put(
mock_client,
all_data=all_data,
el={
"filepath": "file-001",
"raw_upload_location": "http://storage-url-001",
},
)
assert success_response == {
"status_code": 204,
"filepath": "file-001",
"succeeded": True,
}

@pytest.mark.asyncio
async def test_send_single_upload_put_fail_401(self, mocker):
mock_client = MagicMock()

async def side_effect(presigned_put, data):
if presigned_put == "http://storage-url-002":
return httpx.Response(status_code=401)

mock_client.put.side_effect = side_effect

all_data = {
"file-001": {"some": "data", "id": "1"},
"file-002": {"some": "data", "id": "2"},
"file-003": {"some": "data", "id": "3"},
}

fail_401_response = await send_single_upload_put(
mock_client,
all_data=all_data,
el={
"filepath": "file-002",
"raw_upload_location": "http://storage-url-002",
},
)
assert fail_401_response == {
"status_code": 401,
"exception": None,
"filepath": "file-002",
"succeeded": False,
}

@pytest.mark.asyncio
async def test_send_single_upload_put_fail_exception(self, mocker):
mock_client = MagicMock()

async def side_effect(presigned_put, data):
if presigned_put == "http://storage-url-003":
raise httpx.HTTPError("Some error occured in the request")

mock_client.put.side_effect = side_effect

all_data = {
"file-001": {"some": "data", "id": "1"},
"file-002": {"some": "data", "id": "2"},
"file-003": {"some": "data", "id": "3"},
}

fail_401_response = await send_single_upload_put(
mock_client,
all_data=all_data,
el={
"filepath": "file-003",
"raw_upload_location": "http://storage-url-003",
},
)
assert fail_401_response == {
"status_code": None,
"exception": httpx.HTTPError,
"filepath": "file-003",
"succeeded": False,
}

@pytest.mark.asyncio
@pytest.mark.parametrize(
"finish_endpoint_response,expected",
Expand Down