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

Drop cache column from repos table #80

Merged
merged 1 commit into from
Aug 16, 2023
Merged

Conversation

joseph-sentry
Copy link
Contributor

@joseph-sentry joseph-sentry commented Aug 14, 2023

Purpose/Motivation

The cache column is being removed to an issue where it becomes corrupted and it is impossible to run any queries on the row where the column is corrupted.

I think this actually needs to get merged first (this should probably be merged already): #53

Links to relevant tickets

codecov/engineering-team#92

What does this PR do?

  • Remove cache field from Repository model
  • Remove cache from read_only_fields in RepositoryAdmin
  • Create migration for the above using makemigrations

@scott-codecov
Copy link
Contributor

Trent reminded me that we're still running https://github.com/codecov/codecov.io in production to serve some old public API v1 endpoints. We'll need to make sure those don't also rely on repos.cache in any way.

In the meantime I wonder if it's possible to remove the field from the model definition but keep the column for now? This would mean Django would stop querying it but leave codecov.io working as-is. I'm honestly not sure if that's possible with Django - seems like it should be.

@codecov-staging
Copy link

codecov-staging bot commented Aug 15, 2023

Codecov Report

Patch coverage is 100.00% of modified lines.

Files Changed Coverage
core/admin.py ø
core/models.py ø
core/migrations/0033_remove_repository_cache.py 100.00%

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@joseph-sentry joseph-sentry marked this pull request as ready for review August 15, 2023 18:06
@codecov
Copy link

codecov bot commented Aug 15, 2023

Codecov Report

Merging #80 (bc3104c) into main (5308c45) will not change coverage.
The diff coverage is 100.00%.

@@          Coverage Diff          @@
##            main     #80   +/-   ##
=====================================
  Coverage   95.26   95.26           
=====================================
  Files        695     696    +1     
  Lines      14776   14780    +4     
=====================================
+ Hits       14075   14079    +4     
  Misses       701     701           
Flag Coverage Δ
unit 95.20% <100.00%> (+<0.01%) ⬆️
unit-latest-uploader 95.20% <100.00%> (+<0.01%) ⬆️

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

Files Changed Coverage Δ
core/admin.py 76.27% <ø> (ø)
core/models.py 94.53% <ø> (-0.03%) ⬇️
core/migrations/0034_remove_repository_cache.py 100.00% <100.00%> (ø)

@joseph-sentry joseph-sentry force-pushed the joseph/drop-cache-column branch 2 times, most recently from 8b1658c to 91266b7 Compare August 16, 2023 09:30
This commit adds a migration to remove the repos.cache
field from Django but not from the table. It does this
by using state_operations to remove the field while
running a noop SQL migration.

Signed-off-by: joseph-sentry <joseph.sawaya@sentry.io>
@joseph-sentry joseph-sentry merged commit b7b9a48 into main Aug 16, 2023
9 of 10 checks passed
@joseph-sentry joseph-sentry deleted the joseph/drop-cache-column branch August 16, 2023 17:38
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.

None yet

2 participants