Clean up some carry-forward related code #738

Swatinem · 2024-09-25T09:24:40Z

This just moves some code around, removes excessive logging and metrics, and aims to make things a bit more readable.

Feel free to review this with "ignore whitespace", as it does some re-indenting.

This just moves some code around, removes excessive logging and metrics, and aims to make things a bit more readable.

codecov · 2024-09-25T09:30:49Z

Codecov Report

Attention: Patch coverage is 98.21429% with 2 lines in your changes missing coverage. Please review.

Project coverage is 98.06%. Comparing base (6f7044e) to head (4a06290).
Report is 3 commits behind head on main.

✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
services/report/__init__.py	97.61%	1 Missing ⚠️
services/report/raw_upload_processor.py	97.05%	1 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #738   +/-   ##
=======================================
  Coverage   98.05%   98.06%           
=======================================
  Files         434      434           
  Lines       36466    36403   -63     
=======================================
- Hits        35758    35699   -59     
+ Misses        708      704    -4

Flag	Coverage Δ
integration	`98.06% <98.21%> (+<0.01%)`	⬆️
unit	`98.06% <98.21%> (+<0.01%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components	Coverage Δ
NonTestCode	`95.97% <97.91%> (+0.01%)`	⬆️
OutsideTasks	`98.08% <98.05%> (+0.01%)`	⬆️

Files with missing lines	Coverage Δ
helpers/labels.py	`100.00% <100.00%> (ø)`
services/path_fixer/__init__.py	`100.00% <100.00%> (ø)`
services/path_fixer/tests/unit/test_path_fixer.py	`100.00% <ø> (ø)`
services/report/tests/unit/test_sessions.py	`100.00% <100.00%> (ø)`
.../report/tests/unit/test_sessions_encoded_labels.py	`100.00% <100.00%> (ø)`
services/repository.py	`96.76% <ø> (ø)`
services/tests/test_report.py	`100.00% <100.00%> (ø)`
tasks/tests/integration/test_upload_e2e.py	`100.00% <100.00%> (ø)`
tasks/upload_finisher.py	`97.14% <100.00%> (-0.02%)`	⬇️
services/report/__init__.py	`96.76% <97.61%> (-0.03%)`	⬇️
... and 1 more

codecov-staging · 2024-09-25T09:30:52Z

Codecov Report

Attention: Patch coverage is 98.21429% with 2 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
services/report/__init__.py	97.61%	1 Missing ⚠️
services/report/raw_upload_processor.py	97.05%	1 Missing ⚠️

@@           Coverage Diff           @@
##             main     #738   +/-   ##
=======================================
  Coverage   98.05%   98.06%           
=======================================
  Files         434      434           
  Lines       36466    36403   -63     
=======================================
- Hits        35758    35699   -59     
+ Misses        708      704    -4

Flag	Coverage Δ
integration	`98.06% <98.21%> (+<0.01%)`	⬆️
unit	`98.06% <98.21%> (+<0.01%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components	Coverage Δ
NonTestCode	`95.97% <97.91%> (+0.01%)`	⬆️
OutsideTasks	`98.08% <98.05%> (+0.01%)`	⬆️

Files with missing lines	Coverage Δ
helpers/labels.py	`100.00% <100.00%> (ø)`
services/path_fixer/__init__.py	`100.00% <100.00%> (ø)`
services/path_fixer/tests/unit/test_path_fixer.py	`100.00% <ø> (ø)`
services/report/tests/unit/test_sessions.py	`100.00% <100.00%> (ø)`
.../report/tests/unit/test_sessions_encoded_labels.py	`100.00% <100.00%> (ø)`
services/repository.py	`96.76% <ø> (ø)`
services/tests/test_report.py	`100.00% <100.00%> (ø)`
tasks/tests/integration/test_upload_e2e.py	`100.00% <100.00%> (ø)`
tasks/upload_finisher.py	`97.14% <100.00%> (-0.02%)`	⬇️
services/report/__init__.py	`96.76% <97.61%> (-0.03%)`	⬇️
... and 1 more

codecov-qa · 2024-09-25T09:31:01Z

Codecov Report

Attention: Patch coverage is 98.21429% with 2 lines in your changes missing coverage. Please review.

Project coverage is 98.06%. Comparing base (6f7044e) to head (4a06290).
Report is 3 commits behind head on main.

✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
services/report/__init__.py	97.61%	1 Missing ⚠️
services/report/raw_upload_processor.py	97.05%	1 Missing ⚠️

@@           Coverage Diff           @@
##             main     #738   +/-   ##
=======================================
  Coverage   98.05%   98.06%           
=======================================
  Files         434      434           
  Lines       36466    36403   -63     
=======================================
- Hits        35758    35699   -59     
+ Misses        708      704    -4

Flag	Coverage Δ
integration	`98.06% <98.21%> (+<0.01%)`	⬆️
unit	`98.06% <98.21%> (+<0.01%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components	Coverage Δ
NonTestCode	`95.97% <97.91%> (+0.01%)`	⬆️
OutsideTasks	`98.08% <98.05%> (+0.01%)`	⬆️

Files with missing lines	Coverage Δ
helpers/labels.py	`100.00% <100.00%> (ø)`
services/path_fixer/__init__.py	`100.00% <100.00%> (ø)`
services/path_fixer/tests/unit/test_path_fixer.py	`100.00% <ø> (ø)`
services/report/tests/unit/test_sessions.py	`100.00% <100.00%> (ø)`
.../report/tests/unit/test_sessions_encoded_labels.py	`100.00% <100.00%> (ø)`
services/repository.py	`96.76% <ø> (ø)`
services/tests/test_report.py	`100.00% <100.00%> (ø)`
tasks/tests/integration/test_upload_e2e.py	`100.00% <100.00%> (ø)`
tasks/upload_finisher.py	`97.14% <100.00%> (-0.02%)`	⬇️
services/report/__init__.py	`96.76% <97.61%> (-0.03%)`	⬇️
... and 1 more

codecov-public-qa · 2024-09-25T09:31:08Z

Codecov Report

Attention: Patch coverage is 98.21429% with 2 lines in your changes missing coverage. Please review.

Project coverage is 98.06%. Comparing base (6f7044e) to head (4a06290).

✅ All tests successful. No failed tests found.

@@           Coverage Diff           @@
##             main     #738   +/-   ##
=======================================
  Coverage   98.05%   98.06%           
=======================================
  Files         434      434           
  Lines       36466    36403   -63     
=======================================
- Hits        35758    35699   -59     
+ Misses        708      704    -4

Flag	Coverage Δ
integration	`98.06% <98.21%> (+<0.01%)`	⬆️
unit	`98.06% <98.21%> (+<0.01%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components	Coverage Δ
NonTestCode	`95.97% <97.91%> (+0.01%)`	⬆️
OutsideTasks	`98.08% <98.05%> (+0.01%)`	⬆️

Files	Coverage Δ
helpers/labels.py	`100.00% <100.00%> (ø)`
services/path_fixer/__init__.py	`100.00% <100.00%> (ø)`
services/path_fixer/tests/unit/test_path_fixer.py	`100.00% <ø> (ø)`
services/report/tests/unit/test_sessions.py	`100.00% <100.00%> (ø)`
.../report/tests/unit/test_sessions_encoded_labels.py	`100.00% <100.00%> (ø)`
services/repository.py	`96.76% <ø> (ø)`
services/tests/test_report.py	`100.00% <100.00%> (ø)`
tasks/tests/integration/test_upload_e2e.py	`100.00% <100.00%> (ø)`
tasks/upload_finisher.py	`97.14% <100.00%> (-0.02%)`	⬇️
services/report/__init__.py	`96.76% <97.61%> (-0.03%)`	⬇️
... and 1 more

michelletran-codecov · 2024-09-25T13:52:35Z

services/report/raw_upload_processor.py

+        if current_filename in skip_files or not report_file.contents:
+            continue


Is this correct? Should it be

Suggested change

if current_filename in skip_files or not report_file.contents:

continue

if report_file.contents and current_filename in skip_files:

continue

we skip the file, either explicitly or because it does not have contents.

Ah, that makes sense. I'm struggling to figure out where we're skipping the file because the content is empty. Is this newly added logic?

I merged the above if report_file.contents with this skipping condition, pretty much early-skipping it.

sentry-io · 2024-10-01T19:40:31Z

Suspect Issues

This pull request was deployed and Sentry observed the following issues:

‼️ XMLSyntaxError: Document is empty, line 1, column 1 (, line 1) app.tasks.upload.UploadProcessor View Issue

_{Did you find this useful? React with a 👍 or 👎}

Clean up some carry-forward related code

d3f8963

This just moves some code around, removes excessive logging and metrics, and aims to make things a bit more readable.

Swatinem requested a review from a team September 25, 2024 09:24

Swatinem self-assigned this Sep 25, 2024

Swatinem added 3 commits September 25, 2024 12:34

fix tests

1c961c6

rename fn

f476274

fix parallel verification

4a06290

michelletran-codecov reviewed Sep 25, 2024

View reviewed changes

michelletran-codecov approved these changes Sep 25, 2024

View reviewed changes

Swatinem added this pull request to the merge queue Sep 26, 2024

Merged via the queue into main with commit f634f37 Sep 26, 2024
26 of 27 checks passed

Swatinem deleted the swatinem/carry-forward branch September 26, 2024 11:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Clean up some carry-forward related code #738

Clean up some carry-forward related code #738

Swatinem commented Sep 25, 2024

codecov bot commented Sep 25, 2024 •

edited

Loading

codecov-staging bot commented Sep 25, 2024 •

edited by codecov-notifications bot

Loading

codecov-qa bot commented Sep 25, 2024 •

edited

Loading

codecov-public-qa bot commented Sep 25, 2024 •

edited

Loading

michelletran-codecov Sep 25, 2024

Swatinem Sep 25, 2024

michelletran-codecov Sep 25, 2024

Swatinem Sep 25, 2024

sentry-io bot commented Oct 1, 2024

		if current_filename in skip_files or not report_file.contents:
		continue

Clean up some carry-forward related code #738

Clean up some carry-forward related code #738

Conversation

Swatinem commented Sep 25, 2024

codecov bot commented Sep 25, 2024 • edited Loading

Codecov Report

codecov-staging bot commented Sep 25, 2024 • edited by codecov-notifications bot Loading

Codecov Report

codecov-qa bot commented Sep 25, 2024 • edited Loading

Codecov Report

codecov-public-qa bot commented Sep 25, 2024 • edited Loading

Codecov Report

michelletran-codecov Sep 25, 2024

Choose a reason for hiding this comment

Swatinem Sep 25, 2024

Choose a reason for hiding this comment

michelletran-codecov Sep 25, 2024

Choose a reason for hiding this comment

Swatinem Sep 25, 2024

Choose a reason for hiding this comment

sentry-io bot commented Oct 1, 2024

Suspect Issues

codecov bot commented Sep 25, 2024 •

edited

Loading

codecov-staging bot commented Sep 25, 2024 •

edited by codecov-notifications bot

Loading

codecov-qa bot commented Sep 25, 2024 •

edited

Loading

codecov-public-qa bot commented Sep 25, 2024 •

edited

Loading