feat: move Pull.flare to storage
#57
Conversation
This commit is the initial work to move `Pull.flare` to storage. This column has crashed the DB in only 1 occasion in the past. Notice that Pull doesn't have `commit` (because multiple commits refer to the same pull). To deal with that I changed the writing of data to have 2 paths: one with commit, and one without the commit. So pulls data will be written in the repo level (in storage). codecov/engineering-team#70
Codecov Report
Changes have been made to critical files, which contain lines commonly executed in production. Learn more @@ Coverage Diff @@
## main #57 +/- ##
==========================================
- Coverage 98.60% 98.59% -0.02%
==========================================
Files 357 357
Lines 25967 26004 +37
==========================================
+ Hits 25606 25639 +33
- Misses 361 365 +4
Flags with carried forward coverage won't be shown. Click here to find out more.
|
giovanni-guidini
requested review from
matt-codecov,
dana-yaish and
scott-codecov
| return False | ||
| is_codecov_repo = self.repository.owner.username == "codecov" | ||
| return should_write_data_to_storage_config_check( | ||
| master_switch_key="pull_flare", |
There was a problem hiding this comment.
I see this is referencing a pull_flare config value but is a generic method on the model. I feel like maybe we chatted about this scenario in the past but what if there are multiple ArchiveFields on a single model?
Not a big deal right now but this might require a little reworking if that case does arise in the future.
There was a problem hiding this comment.
Good point. I was honestly thinking of creating 2 should_write... methods in the class, one for each field (and naming them appropriately). Maybe we can create a decorator for the master_switch_key that will populate it correctly and we can reuse the generic function, or something like that.
There are options when the time comes to make a final decision, and I don't think we will need to change the ArchiveField to address this situation. I might be wrong tho 😅
* Emit metric of how long a task was enqueued This is being done mostly because of `label-analysis` task, where we want better understanding of how much time the tasks stay in the queue, and what's the impact of that in overall ATS. But this is a useful metric anyway to fine tune other parameters in celery, and to understand what is the gain of having dedicated queues and all that in hard numbers. So we're doing it for all tasks. The idea is simple: we add a header with the timestamp where the task was created (i.e. added to the queue) and on the other side, when it's executed, we emit a metric with the timedelta. [Coverage-Team #57](Extract time in queue metric for all worker's tasks) * Use isoformat instead of custom format for the header
This commit is the initial work to move
Pull.flareto storage.This column has crashed the DB in only 1 occasion in the past.
Notice that Pull doesn't have
commit(because multiple commits refer to the same pull).To deal with that I changed the writing of data to have 2 paths: one with commit, and one
without the commit. So pulls data will be written in the repo level (in storage).
codecov/engineering-team#70
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.