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.

Sign up for GitHub

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

Jump to bottom

clean up unnecessary extra db queries in MetricContext #443

Merged
merged 2 commits into from
May 9, 2024
Merged

clean up unnecessary extra db queries in MetricContext #443

merged 2 commits into from
May 9, 2024

Conversation

matt-codecov
Copy link
Contributor

@matt-codecov matt-codecov commented May 8, 2024 •
edited

clean up unnecessary extra db queries in MetricContext

Stack created with Sapling. Best reviewed with ReviewStack.

@matt-codecov matt-codecov mentioned this pull request May 8, 2024
Closed
@matt-codecov matt-codecov requested a review from a team May 8, 2024 22:16
@codecov Codecov
Copy link

codecov bot commented May 8, 2024 •
edited

Codecov Report

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

Project coverage is 97.37%. Comparing base (f2ce06f) to head (e1460e0).

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

✅ All tests successful. No failed tests found.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #443   +/-   ##
=======================================
  Coverage   97.36%   97.37%           
=======================================
  Files         430      430           
  Lines       34319    34293   -26     
=======================================
- Hits        33415    33392   -23     
+ Misses        904      901    -3
Flag Coverage Δ
integration 97.34% <75.00%> (+<0.01%) ⬆️
latest-uploader-overall 97.34% <75.00%> (+<0.01%) ⬆️
unit 97.34% <75.00%> (+<0.01%) ⬆️

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

Components Coverage Δ
NonTestCode 94.60% <50.00%> (+0.01%) ⬆️
OutsideTasks 97.50% <75.00%> (+0.01%) ⬆️
Files Coverage Δ
django_scaffold/settings.py 0.00% <ø> (ø)
helpers/tests/unit/test_telemetry.py 100.00% <100.00%> (ø)
helpers/telemetry.py Critical 96.82% <50.00%> (+2.99%) ⬆️
Related Entrypoints
run/app.tasks.bundle_analysis.BundleAnalysisProcessor
run/app.tasks.upload.Upload
run/app.tasks.status.SetError
run/app.tasks.notify.Notify
run/app.tasks.timeseries.save_commit_measurements
run/app.tasks.profiling.normalizer
run/app.tasks.pulls.Sync
run/app.tasks.compute_comparison.ComputeComparison
run/app.tasks.upload.UploadFinisher
run/app.tasks.upload.UploadProcessor
run/app.tasks.commit_update.CommitUpdate
run/app.tasks.upload.PreProcessUpload
run/app.tasks.profiling.collection
run/app.tasks.upload.ParallelVerification
run/app.tasks.sync_repo_languages_gql.SyncLanguagesGQL
run/app.tasks.bundle_analysis.BundleAnalysisNotify
run/app.tasks.test_results.TestResultsFinisherTask
run/app.tasks.sync_repo_languages.SyncLanguages
run/app.cron.profiling.findinguncollected
run/app.tasks.profiling.summarization
run/app.tasks.static_analysis.check_suite
run/app.tasks.sync_repos.SyncRepos
run/app.cron.hourly_check.HourlyCheckTask
run/app.tasks.test_results.TestResultsProcessor
run/app.tasks.new_user_activated.NewUserActivated
run/app.tasks.sync_teams.SyncTeams
run/app.tasks.timeseries.backfill_commits
run/app.cron.plan.TrialExpirationCronTask
run/app.cron.daily.GitHubAppWebhooksCheckTask
run/app.tasks.timeseries.backfill_dataset
run/app.tasks.label_analysis.process_request
run/app.tasks.flush_repo.FlushRepo
run/app.tasks.upload.ManualUploadCompletionTrigger
run/app.cron.daily.BrollyStatsRollupTask

@codecov-notifications
Copy link

codecov-notifications bot commented May 8, 2024 •
edited

Codecov Report

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

✅ All tests successful. No failed tests found.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #443   +/-   ##
=======================================
  Coverage   97.33%   97.34%           
=======================================
  Files         399      399           
  Lines       33629    33603   -26     
=======================================
- Hits        32734    32711   -23     
+ Misses        895      892    -3
Flag Coverage Δ
integration 97.34% <75.00%> (+<0.01%) ⬆️
latest-uploader-overall 97.34% <75.00%> (+<0.01%) ⬆️
unit 97.34% <75.00%> (+<0.01%) ⬆️

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

Components Coverage Δ
NonTestCode 94.58% <50.00%> (+0.01%) ⬆️
OutsideTasks 97.50% <75.00%> (+0.01%) ⬆️
Files Coverage Δ
django_scaffold/settings.py 0.00% <ø> (ø)
helpers/tests/unit/test_telemetry.py 100.00% <100.00%> (ø)
helpers/telemetry.py 95.08% <50.00%> (+2.67%) ⬆️

@codecov-qa
Copy link

codecov-qa bot commented May 8, 2024 •
edited

Codecov Report

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

Project coverage is 97.34%. Comparing base (f2ce06f) to head (e1460e0).

✅ All tests successful. No failed tests found.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #443   +/-   ##
=======================================
  Coverage   97.33%   97.34%           
=======================================
  Files         399      399           
  Lines       33629    33603   -26     
=======================================
- Hits        32734    32711   -23     
+ Misses        895      892    -3
Flag Coverage Δ
integration 97.34% <75.00%> (+<0.01%) ⬆️
latest-uploader-overall 97.34% <75.00%> (+<0.01%) ⬆️
unit 97.34% <75.00%> (+<0.01%) ⬆️

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

Components Coverage Δ
NonTestCode 94.58% <50.00%> (+0.01%) ⬆️
OutsideTasks 97.50% <75.00%> (+0.01%) ⬆️
Files Coverage Δ
django_scaffold/settings.py 0.00% <ø> (ø)
helpers/tests/unit/test_telemetry.py 100.00% <100.00%> (ø)
helpers/telemetry.py 95.08% <50.00%> (+2.67%) ⬆️

@codecov-public-qa Codecov Public QA
Copy link

codecov-public-qa bot commented May 8, 2024 •
edited

Codecov Report

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

Project coverage is 97.34%. Comparing base (f2ce06f) to head (e1460e0).

✅ All tests successful. No failed tests found ☺️

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #443   +/-   ##
=======================================
  Coverage   97.33%   97.34%           
=======================================
  Files         399      399           
  Lines       33629    33603   -26     
=======================================
- Hits        32734    32711   -23     
+ Misses        895      892    -3
Flag Coverage Δ
integration 97.34% <75.00%> (+<0.01%) ⬆️
latest-uploader-overall 97.34% <75.00%> (+<0.01%) ⬆️
unit 97.34% <75.00%> (+<0.01%) ⬆️

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

Components Coverage Δ
NonTestCode 94.58% <50.00%> (+0.01%) ⬆️
OutsideTasks 97.50% <75.00%> (+0.01%) ⬆️
Files Coverage Δ
django_scaffold/settings.py 0.00% <ø> (ø)
helpers/tests/unit/test_telemetry.py 100.00% <100.00%> (ø)
helpers/telemetry.py 95.08% <50.00%> (+2.67%) ⬆️

adrian-codecov
adrian-codecov reviewed May 9, 2024
View reviewed changes
helpers/telemetry.py

if self.commit_sha:
if self.commit_sha and not self.commit_id:
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you have a sha but not an id?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i think so, don't remember where i saw this. i think it was maybe like, a webhook or GH API response or something that didn't include our DB id but did include the sha

@matt-codecov matt-codecov added this pull request to the merge queue May 9, 2024
Merged via the queue into main with commit 478837c May 9, 2024
25 of 29 checks passed
@matt-codecov matt-codecov deleted the pr443 branch May 9, 2024 17:34
@michelletran-codecov michelletran-codecov mentioned this pull request May 15, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@adrian-codecov adrian-codecov adrian-codecov approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@matt-codecov @adrian-codecov