Conversation
Update fideslang dependency to use the feat/add-data-purposes-to-dataset-models branch which adds data_purposes at dataset, collection, field, and sub-field levels. Dependency: ethyca/fideslang#39 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…rect-reference error Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Greptile SummaryThis PR introduces
Confidence Score: 1/5
Important Files Changed
Reviews (1): Last reviewed commit: "Merge branch 'main' into feat/dataset-da..." | Re-trigger Greptile |
requirements.txt
Outdated
| @@ -0,0 +1 @@ | |||
| fideslang @ git+https://github.com/ethyca/fideslang.git@feat/add-data-purposes-to-dataset-models | |||
There was a problem hiding this comment.
Dependency override has no effect
This new requirements.txt file is not referenced anywhere in the project's install configuration, CI workflows, or Makefile. The actual project dependency is declared in pyproject.toml (line 58: "fideslang==3.1.4a1") and locked in uv.lock. Creating a standalone requirements.txt at the repo root does not override those — uv sync and uv install will continue installing fideslang==3.1.4a1 from PyPI.
To actually swap in the unreleased branch build, you would need to either:
- Update
pyproject.tomlto use the git reference:"fideslang @ git+https://github.com/ethyca/fideslang.git@feat/add-data-purposes-to-dataset-models" - Regenerate
uv.lockwithuv lock
The CI check in .github/workflows/publish_docs.yaml that uses requirements.txt only reads docs/fides/requirements.txt, not the root-level file added here.
requirements.txt
Outdated
| @@ -0,0 +1 @@ | |||
| fideslang @ git+https://github.com/ethyca/fideslang.git@feat/add-data-purposes-to-dataset-models | |||
There was a problem hiding this comment.
Branch reference is non-deterministic
The dependency is pinned to a branch name (@feat/add-data-purposes-to-dataset-models) rather than a specific commit SHA. This means different installs at different points in time may pick up different code if new commits are pushed to that branch, making builds non-reproducible.
When the intent is to pin to a specific pre-release state, use a commit SHA instead:
fideslang @ git+https://github.com/ethyca/fideslang.git@<specific-commit-sha>
The PR description already acknowledges this needs to be replaced with the final release version before merge, but documenting the SHA in the meantime prevents accidental drift during development/review.
There was a problem hiding this comment.
Code Review
This PR introduces a single file change — a new root-level requirements.txt that pins fideslang to a feature branch in order to get data_purposes field support ahead of its official release.
Critical (Must Fix)
1. Mutable branch reference is non-reproducible.
The pin fideslang @ git+...@feat/add-data-purposes-to-dataset-models resolves to branch HEAD at install time. Any force-push or new commit to that branch silently changes what gets installed. Pin to an immutable commit SHA until the release is available, then update to the release version before merging.
2. Conflicting dependency declaration.
pyproject.toml already declares fideslang==3.1.4a1 as the authoritative dependency. This new requirements.txt at the repo root creates a second, conflicting source of truth. CI workflows in this repo only reference docs/fides/requirements.txt and qa/requirements.txt — there is no evidence this root-level file is consumed by any pipeline, Docker build, or install step. The net effect is that local environments that manually run pip install -r requirements.txt will get a different fideslang than environments driven by pip install -e . (i.e., CI and production). If the intent is to override the dependency for development purposes, the right place is pyproject.toml directly.
Suggestions
- The PR description explicitly notes this must not be merged until
ethyca/fideslang#39is released. Consider adding a draft status or a blocking label to prevent accidental early merge, since there is no automated guard on this. - The CHANGELOG entry is unchecked in the pre-merge checklist — if
data_purposessupport is user-facing, a changelog entry will be needed before merge. - No application code changes accompany the dependency bump. Even if the new field flows through automatically via fideslang's Pydantic models, it would be worth adding or calling out at least one integration test that round-trips
data_purposesat each level (dataset, collection, field, sub-field) to guard against regressions.
Dependency ReviewThe following issues were found:
Snapshot WarningsEnsure that dependencies are being submitted on PR branches and consider enabling retry-on-snapshot-warnings. See the documentation for more information and troubleshooting advice. Scanned Files
|
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The fideslang 3.1.3 bump added a data_purposes field to the Dataset Pydantic model, but the SQLAlchemy model was missing the corresponding column, causing TypeError on dataset create/update. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The customer_dataset was not being normalized through FideslangDataset like stored and upcoming datasets were, causing DeepDiff to detect false changes when fideslang adds new fields (e.g. data_purposes). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Description Of Changes
Update fideslang dependency to version 3.1.3, which includes
data_purposesfield support. This enables purpose-based access control (PBAC) at dataset, collection, field, and sub-field levels via existing dataset CRUD endpoints.Code Changes
pyproject.tomlto pinfideslang==3.1.3(from3.1.4a1)Steps to Confirm
data_purposesat all levels viaPOST /api/v1/dataset/upsertdata_purposespersists at all levelsdata_purposesstill workPre-Merge Checklist
CHANGELOG.mdupdated