refactor: unify dataset endpoints and remove legacy Dataset model#704
Open
SanjeevLakhwani wants to merge 12 commits into
Open
refactor: unify dataset endpoints and remove legacy Dataset model#704SanjeevLakhwani wants to merge 12 commits into
SanjeevLakhwani wants to merge 12 commits into
Conversation
Four bases (/datasets, /datasets_v2, /api/datasets, /api/datasets_v2) all pointed at the same ViewSet with inconsistent sub-path coverage. Unified to /api/datasets only. - Add counts, data_types, data_type_detail, translations, translation_detail as @action methods on DatasetV2ViewSet so the DRF router picks them up under /api/datasets/<id>/... - Remove /datasets and /datasets_v2 root-level path() entries - Remove duplicate router.register for datasets_v2 - Delete now-unreachable dataset_summary, dataset_v2_summary, dataset_v2_data_type_summary, dataset_v2_data_type view functions and their orphaned imports - Update test reverse() names to DRF router-generated names
Drop the Dataset model, its management commands, and all references. DatasetV2 is now the sole dataset representation: - project.datasets (renamed from dv2) exposes DatasetV2 via FK reverse relation - ProjectSerializer.datasets uses DatasetV2Serializer (was datasets_v2) - Migration 0013 made no-op (data already migrated in all deployments) - Migration 0014 drops Dataset table, renames related_name via AlterField Also adds mark_authz_done to DatasetV2ViewSet.data_types action paths (missing call caused 403 on valid and not-found requests).
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #704 +/- ##
===========================================
+ Coverage 93.33% 94.88% +1.55%
===========================================
Files 139 138 -1
Lines 6060 5848 -212
Branches 597 556 -41
===========================================
- Hits 5656 5549 -107
+ Misses 346 245 -101
+ Partials 58 54 -4 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
…atasetTranslation Add migration 0015 with RenameModel ops (renames DB tables chord_datasetv2 → chord_dataset and chord_datasetv2translation → chord_datasettranslation). Update all non-migration Python references across the codebase, including the Resource.get_scope_filters() reverse-accessor update (datasetv2_set → dataset_set, datasetv2__ → dataset__).
…ataset, VALID_DATASET_V2_PRIMARY_CONTACT → VALID_DATASET_PRIMARY_CONTACT)
…t helpers and test cases
Remove duplicate test_dataset_summary / test_dataset_summary_not_found methods that collided after v2 rename; fix over-indented continuation line in test_api.py.
gsfk
approved these changes
Jun 5, 2026
Member
gsfk
left a comment
gsfk
left a comment
There was a problem hiding this comment.
working for me, and all stray v2s are gone.
davidlougheed
requested changes
Jun 5, 2026
|
|
||
| @async_api_view(["GET"]) | ||
| @permission_classes([BentoAllowAny]) | ||
| async def dataset_summary(request: DrfRequest, identifier: str): |
Member
There was a problem hiding this comment.
i'd prefer if these legacy chord endpoints are left as-is. they should be cleaned up in coming versions anyway.
Collaborator
Author
There was a problem hiding this comment.
Why do you want dead code? These endpoints are already covered by DatasetViewSet. Consolidating them was requested on the initial PR, I apologize for not doing it properly on the first pr. The URL routes are also removed in urls.py, so reverting the functions would leave unreachable dead code.
|
|
||
| @async_api_view(["GET"]) | ||
| @permission_classes([BentoAllowAny]) | ||
| async def dataset_v2_summary(request: DrfRequest, identifier: str): |
|
|
||
| @api_view(["GET"]) | ||
| @permission_classes([BentoAllowAny]) | ||
| async def dataset_v2_data_type_summary(request: DrfRequest, identifier: str): |
Member
There was a problem hiding this comment.
i'd prefer if these legacy chord endpoints are left as-is. they should be cleaned up in coming versions anyway.
|
|
||
| @api_view(["GET", "DELETE"]) | ||
| @permission_classes([BentoDeferToHandler]) | ||
| async def dataset_v2_data_type(request: DrfRequest, identifier: str, data_type: str): |
| ] | ||
|
|
||
| operations = [ | ||
| django.contrib.postgres.operations.CreateExtension('pg_trgm'), |
Member
There was a problem hiding this comment.
q: why is an old migration being modified?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Redmine tickets: 2833, 2826
/datasets,/datasets_v2,/api/datasets,/api/datasets_v2) into a single/api/datasetsDatasetmodel, its serializer, and three management commands (create_dataset,list_project_datasets,migrate_datasets_to_v2)DatasetV2→DatasetandDatasetV2Translation→DatasetTranslation(migration0015renames DB tables);DatasetV2Serializer→DatasetSerializer,DatasetV2ViewSet→DatasetViewSetproject.datasetsreverse relation (renamed fromdv2) andProjectSerializer.datasetsnow exposeDatasetdirectly — thedatasets_v2key is gone0013made a no-op (one-time data migration already applied to all deployments); migration0014drops theDatasettable and renames the FKrelated_namemark_authz_doneinDatasetViewSet.data_typesaction that caused 403 on all requests to that endpointpg_trgmextension not created on fresh DBs (migrationpatients/0012now runsCreateExtension('pg_trgm')before first GIN index)Biosamplequeryset byid)