Open
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Adds support for transitioning RLS group subjects from slug-based identifiers to real group IDs, including resolution of stored configs and runtime group matching.
Changes:
- Introduces helpers to detect slug-vs-ID group entries and to decide whether to fetch subject groups by slug or by real ID (or both during mixed/transition cases).
- Updates dataset load/save and data-api request preparation paths to resolve group slugs into real IDs and to fetch allowed groups accordingly.
- Adds/extends unit + integration tests and new resolved v2 RLS config fixture.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| lib/dl_rls/dl_rls_tests/unit/test_utils.py | Adds unit tests for is_slug and rls_uses_real_group_ids. |
| lib/dl_rls/dl_rls_tests/unit/test_rls.py | Adds RLS matching tests covering slug-based and real-ID-based group entries. |
| lib/dl_rls/dl_rls/utils.py | Adds is_slug + rls_uses_real_group_ids utilities used by API layers. |
| lib/dl_rls/dl_rls/testing/rls_configs/dl_api_lib_group_config_resolved_v2.json | Adds a v2 fixture with resolved real group ID. |
| lib/dl_rls/dl_rls/subject_resolver.py | Makes resolve_group_slug explicitly unimplemented by default. |
| lib/dl_rls/dl_rls/init.py | Exposes selected dl_rls APIs via package-level exports. |
| lib/dl_api_lib_testing/dl_api_lib_testing/app.py | Extends test resolver to support by_id group fetch and slug-to-ID resolution. |
| lib/dl_api_lib/dl_api_lib_tests/db/data_api/test_rls.py | Adds a data-api test case using the resolved v2 group config. |
| lib/dl_api_lib/dl_api_lib_tests/db/control_api/test_rls.py | Adds control-api test validating slug resolution into v2 real IDs on save/load. |
| lib/dl_api_lib/dl_api_lib/app/data_api/resources/dataset/base.py | Updates runtime group resolution to fetch groups by slug vs ID depending on config. |
| lib/dl_api_lib/dl_api_lib/api_common/dataset_loader.py | Resolves group slugs to real IDs when persisting dataset RLS changes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
lib/dl_api_lib/dl_api_lib/app/data_api/resources/dataset/base.py
Outdated
Show resolved
Hide resolved
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
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.
No description provided.