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.

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

fix: add report file name to logs #510

Merged
merged 2 commits into from
Jun 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 3 additions & 1 deletion helpers/exceptions.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
class ReportExpiredException(Exception):
pass
def __init__(self, message=None, filename=None) -> None:
super().__init__(message)
self.filename = filename


class ReportEmptyError(Exception):
Expand Down
9 changes: 7 additions & 2 deletions services/report/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -1016,11 +1016,16 @@ def build_report_from_raw_content(
raw_report=result.raw_report,
upload_obj=upload,
)
except ReportExpiredException:
except ReportExpiredException as r:
log.info(
"Report %s is expired",
reportid,
extra=dict(repoid=commit.repoid, commit=commit.commitid),
extra=dict(
repoid=commit.repoid,
commit=commit.commitid,
archive_path=archive_url,
file_name=r.filename,
),
)
return ProcessingResult(
report=None,
Expand Down
12 changes: 8 additions & 4 deletions services/report/raw_upload_processor.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
from shared.utils.sessions import Session, SessionType

from database.models.reports import Upload
from helpers.exceptions import ReportEmptyError
from helpers.exceptions import ReportEmptyError, ReportExpiredException
from helpers.labels import get_all_report_labels, get_labels_per_session
from rollouts import USE_LABEL_INDEX_IN_REPORT_PROCESSING_BY_REPO_ID
from services.path_fixer import PathFixer
Expand Down Expand Up @@ -155,9 +155,13 @@ def process_raw_upload(
path_fixer_to_use,
should_use_encoded_labels,
)
report = process_report(
report=report_file, report_builder=report_builder_to_use
)
try:
report = process_report(
report=report_file, report_builder=report_builder_to_use
)
except ReportExpiredException as r:
r.filename = current_filename
Copy link
Contributor

Choose a reason for hiding this comment

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

Test uncovered in case we want to make a test for this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed!

raise
if report:
if should_use_encoded_labels:
# Copies the labels from report into temporary_report
Expand Down
38 changes: 37 additions & 1 deletion services/report/tests/unit/test_process.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,11 @@
from shared.utils.sessions import Session, SessionType
from shared.yaml import UserYaml

from helpers.exceptions import CorruptRawReportError, ReportEmptyError
from helpers.exceptions import (
CorruptRawReportError,
ReportEmptyError,
ReportExpiredException,
)
from services.report import raw_upload_processor as process
from services.report.parser import LegacyReportParser
from services.report.parser.types import LegacyParsedRawReport, ParsedUploadedReportFile
Expand Down Expand Up @@ -984,6 +988,38 @@ def test_process_raw_upload_multiple_raw_reports(self, mocker):
assert sorted(res.sessions.keys()) == [0, 1]
assert res.sessions[1] == session

def test_process_raw_upload_expired_report(self, mocker):
filename = "/Users/path/to/app.coverage.txt"
uploaded_reports = LegacyParsedRawReport(
toc=None,
env=None,
report_fixes=None,
uploaded_files=[
ParsedUploadedReportFile(
filename="/Users/path/to/app.coverage.txt",
file_contents=BytesIO("<data>".encode()),
),
],
)
mocker.patch.object(
process,
"process_report",
side_effect=[
ReportExpiredException(),
],
)
session = Session()
with pytest.raises(ReportExpiredException) as e:
_ = process.process_raw_upload(
UserYaml({}),
None,
uploaded_reports,
["flag_one", "flag_two"],
session=session,
)

assert e.value.filename == filename


class TestProcessRawUploadCarryforwardFlags(BaseTestCase):
def _generate_sample_report(self) -> EditableReport:
Expand Down
Loading