Conversation
…n/create-archive-collector
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #651 +/- ##
==========================================
- Coverage 92.97% 92.44% -0.54%
==========================================
Files 1293 1295 +2
Lines 47318 47608 +290
Branches 1592 1592
==========================================
+ Hits 43996 44012 +16
- Misses 3013 3287 +274
Partials 309 309
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
…h within bucket instead
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| "export_path": f"exports/{owner_id}/{export_id}/", | ||
| "export_date": timezone.now().isoformat(), | ||
| "since_date": since_date.isoformat() if since_date else None, | ||
| "days_exported": EXPORT_DAYS_DEFAULT, |
There was a problem hiding this comment.
Manifest days_exported hardcoded, may mismatch actual export
Low Severity
The manifest's days_exported field is hardcoded to EXPORT_DAYS_DEFAULT (60), but the actual since_date used for filtering data may differ. When the export record specifies a custom since_date, the manifest will claim 60 days of data were exported while the actual export may contain more or fewer days. This inconsistency between since_date and days_exported could mislead users about the actual data coverage.
There was a problem hiding this comment.
Not important, in practice they'll be the same
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| *, | ||
| owner_id: int, | ||
| export_id: int, | ||
| user_id: int | None = None, |
There was a problem hiding this comment.
Unused user_id parameter never updates export record
Low Severity
The user_id parameter is accepted and documented as "Optional user ID who triggered the export" but is never used. The OwnerExport model has a created_by field (ForeignKey to User) intended to track who triggered the export, but the task never updates this field with the provided user_id. The parameter is passed from TaskService.export_owner_data to ExportOwnerTask.run_impl but then completely ignored, resulting in lost audit trail information about who initiated the export.
Additional Locations (1)
There was a problem hiding this comment.
Is user_id for future use?
There was a problem hiding this comment.
Mostly generic stat stuff; we can change it as we go
| try: | ||
| export = OwnerExport.objects.get(id=export_id) | ||
| except OwnerExport.DoesNotExist: | ||
| log.error("OwnerExport %d not found", export_id) |
There was a problem hiding this comment.
Should we change export's status to failed here too?
There was a problem hiding this comment.
Mmm in this case the export won't exist, so there's nothing to set its status to 😉
2 participants
This is the PR that puts all data export PRs together. Calling all data export fns for sql and archive exports, creating a fn that calls the task via API, addressing size + scale issues. Should be reviewed after #650
https://linear.app/getsentry/issue/CCMRG-2008/create-owner-data-export-file
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.
Note
Implements end-to-end owner data export via Celery tasks and updates shared config/routing to support them.
ExportOwner,ExportOwnerSQL,ExportOwnerArchives,ExportOwnerFinalize,ExportOwnerCleanuporchestrated with a chord; generates SQL dumps, collects archives, builds tarball + manifest, presigns download URL, and cleans up on failureTaskService.export_owner_dataadded to triggerexport_ownerworkflowapp.tasks.export_owner.*to a configurable queue;TaskConfigGroupgainsexport_ownersince_datefiltering and callable defaults; adds TimescaleDB export wrapper with date supportWritten by Cursor Bugbot for commit 1c36898. This will update automatically on new commits. Configure here.