[CDF-27106] ✨ Add Data Product Versions (Alpha)#2536
Conversation
Implement CRUD support for data product versions, a sub-resource of data products. Reuses the DATA_PRODUCTS feature flag.
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. |
|
/gemini review |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
The pull request introduces CRUD support for Data Product Versions, including new API client methods, Pydantic resource models, and corresponding CRUD operations. The implementation adheres to the repository's style guide, utilizing strong typing with Pydantic models and providing clear structure for API interactions. Test fixtures have also been added to cover the new functionality. The logic for handling API limitations, such as retrieving all versions to perform updates or deletions, is correctly implemented.
Co-authored-by: Cursor <cursoragent@cursor.com>
The Data Products API is not available on the test server, so skip DataProductVersionCRUD alongside DataProductCRUD.
☂️ Python Coverage
Overall Coverage
New Files
Modified Files
|
… versioning The API now uses a user-specified semantic version string (major.minor.patch) instead of a server-assigned integer versionId. This simplifies the identifier to (dataProductExternalId, version) and removes the need for reverse-lookups in the CRUD layer. Extracts a reusable SemanticVersion annotated type.
…lass pagination, simplify CRUD update
…f-27106/add-data-product-versions
doctrino
left a comment
doctrino
left a comment
There was a problem hiding this comment.
In addition to the comments:
- Remember to make an exception in the smoke tests, ref #2525 (comment)
- Update the approval client configuration, ref https://github.com/cognitedata/toolkit/blob/main/tests/test_unit/approval_client/config.py#L1341
| container_fields: ClassVar[frozenset[str]] = frozenset() | ||
|
|
||
| def as_update(self, mode: Literal["patch", "replace"]) -> dict[str, Any]: | ||
| raise NotImplementedError("Data product version updates use a custom format via the CRUD layer.") |
There was a problem hiding this comment.
Why not use the builint as update implementation?
There was a problem hiding this comment.
Had to overload it because of the nested {"modify": {...}} pattern for terms and dataModel.views
cognite_toolkit/_cdf_tk/cruds/_resource_cruds/data_product_version.py
Outdated
Show resolved
Hide resolved
Co-authored-by: Anders Albert <60234212+doctrino@users.noreply.github.com>
…f-27106/add-data-product-versions # Conflicts: # tests_smoke/test_client/test_cdf_resource_api.py
…com/cognitedata/toolkit into cdf-27106/add-data-product-versions # Conflicts: # cognite_toolkit/_cdf_tk/client/api/data_product_versions.py
doctrino
left a comment
doctrino
left a comment
There was a problem hiding this comment.
Two optional comments.
| def _resolve( | ||
| self, method: APIMethod, **kwargs: str | ||
| ) -> tuple[Literal["GET", "POST", "PUT", "PATCH", "DELETE"], str]: |
There was a problem hiding this comment.
This is my personal opinion, feel free to ignore it if you disagree.
I think this method just obfuscate what is going on where it is used. Especially kwargs is very tricy to understand. I would just do this manually everywhere it is used. Or at least change kwargs with external_id and optionally version.
There was a problem hiding this comment.
Agreed, I had my doubts too. Fixed.
| # The versions update API uses nested {set}/{setNull}/{modify} operators | ||
| # instead of a flat body, so we must build the payload manually. |
There was a problem hiding this comment.
I think this is something we can give feeback on to the API team. This update is deviating significantly from other updates with very nested structure. For example, HostedExtractros have a similar issue and solves it in a more standardized way.
Co-authored-by: Anders Albert <60234212+doctrino@users.noreply.github.com>
…lper Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
2 participants
Description
Implement CRUD support for data product versions, a sub-resource of data products.
Versions are nested under
/dataproducts/{externalId}/versionsand use semantic versioning. Reuses the existing DATA_PRODUCTS feature flag.Bump
Changelog
Added