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

Create CleanLabelIndexTask #205

Merged
merged 9 commits into from
Dec 11, 2023

Conversation

giovanni-guidini
Copy link
Contributor

This task is designed to cleanup the labels_index of reports.
As they are carried forward, and tests are renamed, some indexes
will point to labels that are not used anymore. This will cause
the index to grow indefinetely and the lower indexes will eventually
not be used anymore.

THe task will scan a report and make sure all indexes are being used,
by modifying both the labels_index and the labels in the report.

These change don't schedule the task, only create it.

This task is designed to cleanup the labels_index of reports.
As they are carried forward, and tests are renamed, some indexes
will point to labels that are not used anymore. This will cause
the index to grow indefinetely and the lower indexes will eventually
not be used anymore.

THe task will scan a report and make sure all indexes are being used,
by modifying both the labels_index and the labels in the report.
@codecov-qa
Copy link

codecov-qa bot commented Dec 5, 2023

Codecov Report

Merging #205 (2589c10) into main (bba1a7c) will increase coverage by 0.00%.
The diff coverage is 98.98%.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff            @@
##             main     #205    +/-   ##
========================================
  Coverage   98.36%   98.36%            
========================================
  Files         356      358     +2     
  Lines       28537    28828   +291     
========================================
+ Hits        28069    28357   +288     
- Misses        468      471     +3     
Flag Coverage Δ
integration 98.36% <98.98%> (+<0.01%) ⬆️
latest-uploader-overall 98.36% <98.98%> (+<0.01%) ⬆️
unit 98.36% <98.98%> (+<0.01%) ⬆️

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

Components Coverage Δ
NonTestCode 96.79% <100.00%> (+0.03%) ⬆️
OutsideTasks 98.14% <ø> (ø)
Files Coverage Δ
tasks/__init__.py 100.00% <100.00%> (ø)
tasks/notify.py 98.37% <100.00%> (+<0.01%) ⬆️
tasks/upload.py 99.07% <100.00%> (ø)
tasks/upload_clean_labels_index.py 100.00% <100.00%> (ø)
tasks/upload_processor.py 99.39% <100.00%> (+<0.01%) ⬆️
tasks/tests/unit/test_clean_labels_index.py 98.36% <98.36%> (ø)

Copy link

codecov bot commented Dec 5, 2023

Codecov Report

Merging #205 (2589c10) into main (bba1a7c) will increase coverage by 0.00%.
The diff coverage is 98.98%.

Changes have been made to critical files, which contain lines commonly executed in production. Learn more

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff            @@
##             main     #205    +/-   ##
========================================
  Coverage   98.32%   98.33%            
========================================
  Files         387      389     +2     
  Lines       29232    29524   +292     
========================================
+ Hits        28743    29032   +289     
- Misses        489      492     +3     
Flag Coverage Δ
integration 98.36% <98.98%> (+<0.01%) ⬆️
latest-uploader-overall 98.36% <98.98%> (+<0.01%) ⬆️
onlysomelabels 98.33% <98.98%> (+<0.01%) ⬆️
unit 98.36% <98.98%> (+<0.01%) ⬆️

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

Components Coverage Δ
NonTestCode 96.68% <100.00%> (+0.03%) ⬆️
OutsideTasks 98.14% <ø> (ø)
Files Coverage Δ
tasks/__init__.py 100.00% <100.00%> (ø)
tasks/notify.py Critical 98.37% <100.00%> (+<0.01%) ⬆️
tasks/upload.py Critical 99.08% <100.00%> (ø)
tasks/upload_clean_labels_index.py 100.00% <100.00%> (ø)
tasks/upload_processor.py Critical 99.39% <100.00%> (+<0.01%) ⬆️
tasks/tests/unit/test_clean_labels_index.py 98.36% <98.36%> (ø)
Related Entrypoints
run/app.tasks.upload.Upload
run/app.tasks.notify.Notify
run/app.tasks.upload.UploadProcessor

@giovanni-guidini giovanni-guidini force-pushed the gio/label-compressed/report-with-label-index branch from da2a563 to ec947a4 Compare December 6, 2023 14:18
Base automatically changed from gio/label-compressed/report-with-label-index to main December 6, 2023 14:32
Copy link

codecov-public-qa bot commented Dec 8, 2023

Codecov Report

Merging #205 (2589c10) into main (bba1a7c) will increase coverage by 0.00%.
The diff coverage is 98.98%.

Impacted file tree graph

@@           Coverage Diff            @@
##             main     #205    +/-   ##
========================================
  Coverage   98.36%   98.36%            
========================================
  Files         356      358     +2     
  Lines       28537    28828   +291     
========================================
+ Hits        28069    28357   +288     
- Misses        468      471     +3     
Flag Coverage Δ
integration 98.36% <98.98%> (+<0.01%) ⬆️
latest-uploader-overall 98.36% <98.98%> (+<0.01%) ⬆️
unit 98.36% <98.98%> (+<0.01%) ⬆️

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

Components Coverage Δ
NonTestCode 96.79% <100.00%> (+0.03%) ⬆️
OutsideTasks 98.14% <ø> (ø)
Files Coverage Δ
tasks/__init__.py 100.00% <100.00%> (ø)
tasks/notify.py 98.37% <100.00%> (+<0.01%) ⬆️
tasks/upload.py 99.07% <100.00%> (ø)
tasks/upload_clean_labels_index.py 100.00% <100.00%> (ø)
tasks/upload_processor.py 99.39% <100.00%> (+<0.01%) ⬆️
tasks/tests/unit/test_clean_labels_index.py 98.36% <98.36%> (ø)

giovanni-guidini added a commit that referenced this pull request Dec 8, 2023
If the processed upload includes a flag that is carryforward
AND "labels" mode, we enqueue the CleanLabelsIndexTask.

Depends on #205

I tried to be conservative, but it's possible that - despite
using the flag with labels - no labels are uploaded. It's
also possible that there's nothing to do. But these are small
details.

ps.: To see that indeed we have an "arguments" and "flag" in
"processing_results" I looked at logs for the message
"Scheduling notify task" 😉

closes codecov/engineering-team#768
@giovanni-guidini giovanni-guidini merged commit ebb76ab into main Dec 11, 2023
26 checks passed
@giovanni-guidini giovanni-guidini deleted the gio/label-compressed/clean-index-task branch December 11, 2023 17:45
giovanni-guidini added a commit that referenced this pull request Dec 11, 2023
If the processed upload includes a flag that is carryforward
AND "labels" mode, we enqueue the CleanLabelsIndexTask.

Depends on #205

I tried to be conservative, but it's possible that - despite
using the flag with labels - no labels are uploaded. It's
also possible that there's nothing to do. But these are small
details.

ps.: To see that indeed we have an "arguments" and "flag" in
"processing_results" I looked at logs for the message
"Scheduling notify task" 😉

closes codecov/engineering-team#768
scott-codecov added a commit that referenced this pull request Dec 12, 2023
* main:
  Change default upper section from "header" to "condensed_header" (#209)
  Add logs for some path fixes for each raw upload processing (#135)
  Create CleanLabelIndexTask (#205)
  fix: check if labels before popping (#208)
giovanni-guidini added a commit that referenced this pull request Dec 13, 2023
If the processed upload includes a flag that is carryforward
AND "labels" mode, we enqueue the CleanLabelsIndexTask.

Depends on #205

I tried to be conservative, but it's possible that - despite
using the flag with labels - no labels are uploaded. It's
also possible that there's nothing to do. But these are small
details.

ps.: To see that indeed we have an "arguments" and "flag" in
"processing_results" I looked at logs for the message
"Scheduling notify task" 😉

Thanks @matt-codecov for the suggestions around 
refactoring the trigger logic for CleanLabelsIndex to
make it easier to read and maintain.

closes codecov/engineering-team#768
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants