8000 Further cleanup `UploadArguments` and normalize `flags` by Swatinem · Pull Request #862 · codecov/worker · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content
This repository was archived by the owner on May 5, 2025. It is now read-only.

Further cleanup UploadArguments and normalize flags #862

Merged
merged 1 commit into from
Nov 8, 2024
Merged

Further cleanup UploadArguments and normalize flags #862

merged 1 commit into from
Nov 8, 2024

Conversation

Swatinem
Copy link
Contributor
@Swatinem Swatinem commented Nov 7, 2024

Normalizes the flags, so they are always a list. Otherwise, this uses the UploadArguments type in more places, and cleans up usage of upload_pk in more places.

Another cleanup was removing the backwards compatibility for the old processing_results being passed from the Processor to the Finisher.

@Swatinem Swatinem requested a review from a team November 7, 2024 12:19
@Swatinem Swatinem self-assigned this Nov 7, 2024
@Swatinem Swatinem force-pushed the swatinem/cleanup-flags branch from 17aa192 to 571c280 Compare November 7, 2024 12:24
@codecov Codecov
Copy link
codecov bot commented Nov 7, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 98.71795% with 1 line in your changes missing coverage. Please review.

Project coverage is 97.93%. Comparing base (fa2ef7c) to head (f9d3a88).
Report is 1 commits behind head on main.

✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
tasks/upload_finisher.py 66.66% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #862      +/-   ##
==========================================
- Coverage   97.93%   97.93%   -0.01%     
==========================================
  Files         444      444              
  Lines       35476    35463      -13     
==========================================
- Hits        34744    34731      -13     
  Misses        732      732
Flag Coverage Δ
integration 97.93% <98.71%> (-0.01%) ⬇️
unit 97.93% <98.71%> (-0.01%) ⬇️

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

Components Coverage Δ
NonTestCode 95.95% <98.36%> (ø)
OutsideTasks 97.98% <100.00%> (+<0.01%) ⬆️
Files with missing lines Coverage Δ
services/processing/types.py 100.00% <100.00%> (ø)
services/report/__init__.py 96.96% <100.00%> (-0.05%) ⬇️
services/test_results.py 90.00% <100.00%> (ø)
services/tests/test_report.py 100.00% <ø> (ø)
tasks/bundle_analysis_processor.py 98.43% <100.00%> (-0.03%) ⬇️
tasks/test_results_processor.py 97.15% <100.00%> (-0.03%) ⬇️
tasks/tests/integration/test_upload_e2e.py 100.00% <ø> (ø)
.../tests/unit/test_bundle_analysis_processor_task.py 99.78% <ø> (ø)
...sks/tests/unit/test_test_results_processor_task.py 100.00% <100.00%> (ø)
tasks/tests/unit/test_upload_finisher_task.py 100.00% <100.00%> (ø)
... and 3 more

@codecov-notifications
Copy link
codecov-notifications bot commented Nov 7, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 98.71795% with 1 line in your changes missing coverage. Please review.

✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
tasks/upload_finisher.py 66.66% 1 Missing ⚠️

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #862      +/-   ##
==========================================
- Coverage   97.93%   97.93%   -0.01%     
==========================================
  Files         444      444              
  Lines       35476    35463      -13     
==========================================
- Hits        34744    34731      -13     
  Misses        732      732
Flag Coverage Δ
integration 97.93% <98.71%> (-0.01%) ⬇️
unit 97.93% <98.71%> (-0.01%) ⬇️

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

Components Coverage Δ
NonTestCode 95.95% <98.36%> (ø)
OutsideTasks 97.98% <100.00%> (+<0.01%) ⬆️
Files with missing lines Coverage Δ
services/processing/types.py 100.00% <100.00%> (ø)
services/report/__init__.py 96.96% <100.00%> (-0.05%) ⬇️
services/test_results.py 90.00% <100.00%> (ø)
services/tests/test_report.py 100.00% <ø> (ø)
tasks/bundle_analysis_processor.py 98.43% <100.00%> (-0.03%) ⬇️
tasks/test_results_processor.py 97.15% <100.00%> (-0.03%) ⬇️
tasks/tests/integration/test_upload_e2e.py 100.00% <ø> (ø)
.../tests/unit/test_bundle_analysis_processor_task.py 99.78% <ø> (ø)
...sks/tests/unit/test_test_results_processor_task.py 100.00% <100.00%> (ø)
tasks/tests/unit/test_upload_finisher_task.py 100.00% <100.00%> (ø)
... and 3 more

@codecov-qa
Copy link
codecov-qa bot commented Nov 7, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 98.71795% with 1 line in your changes missing coverage. Please review.

Project coverage is 97.93%. Comparing base (fa2ef7c) to head (f9d3a88).
Report is 1 commits behind head on main.

✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
tasks/upload_finisher.py 66.66% 1 Missing ⚠️

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #862      +/-   ##
==========================================
- Coverage   97.93%   97.93%   -0.01%     
==========================================
  Files         444      444              
  Lines       35476    35463      -13     
==========================================
- Hits        34744    34731      -13     
  Misses        732      732
Flag Coverage Δ
integration 97.93% <98.71%> (-0.01%) ⬇️
unit 97.93% <98.71%> (-0.01%) ⬇️

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

Components Coverage Δ
NonTestCode 95.95% <98.36%> (ø)
OutsideTasks 97.98% <100.00%> (+<0.01%) ⬆️
Files with missing lines Coverage Δ
services/processing/types.py 100.00% <100.00%> (ø)
services/report/__init__.py 96.96% <100.00%> (-0.05%) ⬇️
services/test_results.py 90.00% <100.00%> (ø)
services/tests/test_report.py 100.00% <ø> (ø)
tasks/bundle_analysis_processor.py 98.43% <100.00%> (-0.03%) ⬇️
tasks/test_results_processor.py 97.15% <100.00%> (-0.03%) ⬇️
tasks/tests/integration/test_upload_e2e.py 100.00% <ø> (ø)
.../tests/unit/test_bundle_analysis_processor_task.py 99.78% <ø> (ø)
...sks/tests/unit/test_test_results_processor_task.py 100.00% <100.00%> (ø)
tasks/tests/unit/test_upload_finisher_task.py 100.00% <100.00%> (ø)
... and 3 more

@codecov-public-qa Codecov Public QA
Copy link
codecov-public-qa bot commented Nov 7, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 98.71795% with 1 line in your changes missing coverage. Please review.

Project coverage is 97.93%. Comparing base (fa2ef7c) to head (f9d3a88).

✅ All tests successful. No failed tests found.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #862      +/-   ##
==========================================
- Coverage   97.93%   97.93%   -0.01%     
==========================================
  Files         444      444              
  Lines       35476    35463      -13     
==========================================
- Hits        34744    34731      -13     
  Misses        732      732
Flag Coverage Δ
integration 97.93% <98.71%> (-0.01%) ⬇️
unit 97.93% <98.71%> (-0.01%) ⬇️

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

Components Coverage Δ
NonTestCode 95.95% <98.36%> (ø)
OutsideTasks 97.98% <100.00%> (+<0.01%) ⬆️
Files Coverage Δ
services/processing/types.py 100.00% <100.00%> (ø)
services/report/__init__.py 96.96% <100.00%> (-0.05%) ⬇️
services/test_results.py 90.00% <100.00%> (ø)
services/tests/test_report.py 100.00% <ø> (ø)
tasks/bundle_analysis_processor.py 98.43% <100.00%> (-0.03%) ⬇️
tasks/test_results_processor.py 97.15% <100.00%> (-0.03%) ⬇️
tasks/tests/integration/test_upload_e2e.py 100.00% <ø> (ø)
.../tests/unit/test_bundle_analysis_processor_task.py 99.78% <ø> (ø)
...sks/tests/unit/test_test_results_processor_task.py 100.00% <100.00%> (ø)
tasks/tests/unit/test_upload_finisher_task.py 100.00% <100.00%> (ø)
... and 3 more

@Swatinem Swatinem force-pushed the swatinem/cleanup-flags branch 3 times, most recently from 7a84c55 to c6baa4e Compare November 7, 2024 15:18
adrian-codecov
adrian-codecov reviewed Nov 7, 2024
View reviewed changes
tasks/bundle_analysis_processor.py
repoid=repoid,
commitid=commitid,
commit_yaml=commit_yaml,
params=deepcopy(params),
Copy link
Contributor

Choose a reason for hiding this comment

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

I see we got rid of deepcopy here but I don't see any writing/changes to anything inside of it. I'm curious why we had this in the first place 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed yes, thats why I am cleaning this up :-)

@Adal3n3 Adal3n3 mentioned this pull request Nov 8, 2024
Closed
@Swatinem Swatinem force-pushed the swatinem/cleanup-flags branch from c6baa4e to f3dd525 Compare November 8, 2024 10:47
@Swatinem
Further cleanup UploadArguments and normalize flags
f9d3a88 
Normalizes the `flags`, so they are always a `list`.
Otherwise, this uses the `UploadArguments` type in more places, and cleans up usage of `upload_pk` in more places.

Another cleanup was removing the backwards compatibility for the old `processing_results` being passed from the `Processor` to the `Finisher`.
@Swatinem Swatinem force-pushed the swatinem/cleanup-flags branch from f3dd525 to f9d3a88 Compare November 8, 2024 11:02
@Swatinem Swatinem added this pull request to the merge queue Nov 8, 2024
Merged via the queue into main with commit 5634caa Nov 8, 2024
26 of 27 checks passed
@Swatinem Swatinem deleted the swatinem/cleanup-flags branch November 8, 2024 11:31
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@adrian-codecov adrian-codecov adrian-codecov left review comments

@nora-shap nora-shap nora-shap approved these changes

Assignees

@Swatinem Swatinem

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Swatinem @adrian-codecov @nora-shap
0