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

chore: don't save errors in LabelAnalysisRequest object #99

Merged
merged 1 commit into from
Sep 15, 2023

Conversation

giovanni-guidini
Copy link
Contributor

In the case where we have some missing information we were saving the errors as part of the result dict (in the database).
There's a dedicated table for the larq errors, they should not be saved in the database.
THese changes fix that adding the errors to the dict to return only after savign the real results.

Legal Boilerplate

Look, I get it. The entity doing business as "Sentry" was incorporated in the State of Delaware in 2015 as Functional Software, Inc. In 2022 this entity acquired Codecov and as result Sentry is going to need some rights from me in order to utilize my contributions in this PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Sentry can use, modify, copy, and redistribute my contributions, under Sentry's choice of terms.

In the case where we have some missing information we were saving the errors as part of the result dict (in the database).
There's a dedicated table for the larq errors, they should not be saved in the database.
THese changes fix that adding the errors to the dict to return only after savign the real results.
@codecov
Copy link

codecov bot commented Sep 15, 2023

Codecov Report

Merging #99 (0bfce2d) into main (6d00e5f) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main      #99   +/-   ##
=======================================
  Coverage   98.48%   98.48%           
=======================================
  Files         364      364           
  Lines       26826    26827    +1     
=======================================
+ Hits        26420    26421    +1     
  Misses        406      406           
Flag Coverage Δ
integration 98.45% <100.00%> (+<0.01%) ⬆️
latest-uploader-overall 98.45% <100.00%> (+<0.01%) ⬆️
onlysomelabels 98.48% <100.00%> (+<0.01%) ⬆️
unit 98.45% <100.00%> (+<0.01%) ⬆️

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

Components Coverage Δ
NonTestCode 97.14% <100.00%> (+<0.01%) ⬆️
OutsideTasks 98.25% <ø> (ø)
Files Changed Coverage Δ
tasks/label_analysis.py 99.37% <100.00%> (+<0.01%) ⬆️

This change has been scanned for critical changes. Learn more

@giovanni-guidini giovanni-guidini merged commit 21338dc into main Sep 15, 2023
12 checks passed
@giovanni-guidini giovanni-guidini deleted the gio/dont-save-errors-in-result branch September 15, 2023 17:31
giovanni-guidini added a commit to codecov/codecov-cli that referenced this pull request Sep 19, 2023
codecov/worker#99 had the unwanted and overlooked effect of slightly changing the format
of the response from Codecov label-analysis.

Before the "errors" key was inside the "result" (it shouldn't be)
So the CLI is not surfacing these errors anymore.

These changes fix that and move the log line to a more logical position.
giovanni-guidini added a commit to codecov/codecov-cli that referenced this pull request Sep 20, 2023
codecov/worker#99 had the unwanted and overlooked effect of slightly changing the format
of the response from Codecov label-analysis.

Before the "errors" key was inside the "result" (it shouldn't be)
So the CLI is not surfacing these errors anymore.

These changes fix that and move the log line to a more logical position.
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