feat(Data Modeling): support usedFor=record on containers#2621
feat(Data Modeling): support usedFor=record on containers#2621evertoncolling wants to merge 7 commits into
Conversation
|
This one would close #2391 (which was based on v7) |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #2621 +/- ##
=======================================
Coverage 93.03% 93.03%
=======================================
Files 486 486
Lines 49577 49615 +38
=======================================
+ Hits 46122 46160 +38
Misses 3455 3455
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Code Review
This pull request introduces the ability to filter containers by the 'used_for' attribute in the Data Modeling API. It adds a new 'ContainerUsedFor' type (supporting 'node', 'edge', 'record', and 'all'), updates the 'list' and 'call' methods in both async and sync container APIs to accept this filter, and includes corresponding unit tests. I have no feedback to provide.
|
🦄 |
| Filter containers by `used_for`: | ||
|
|
||
| >>> record_containers = client.data_modeling.containers.list(used_for="record") | ||
| >>> all_containers = client.data_modeling.containers.list(used_for=["all", "record"]) |
There was a problem hiding this comment.
@evertoncolling Won't this exclude "node only" and "edge only" containers? If yes, then we can't name this all_containers, it's just too confusing imo.
Can we, in the SDK, go with node_and_edge instead?
There was a problem hiding this comment.
Yes, that's true!!!
Even I get confused by this from time to time. But I dont think it would be a good idea to "patch" the name of the filter in the SDK. I would rather align with the API, and document it clearly in the docstrings/comments.
I will fix this in a new commit :)
Irrespective of this PR, we may be able to improve this in the API in the future, paving a way for less confusion. That would need some good discussions, based on how do we see the evolution of DMS and Records, the recommended usage patterns, etc.
Description
This PR adds support for creating and querying records containers via the SDK.
ContainerApply.used_forandContainer.used_forto also accept"record", aligning with the v1 API spec (UsedForenum =node | edge | record | all). Exposes a new publicContainerUsedFortype alias.used_forfilter tocontainers.list()andcontainers(...)(iterate), forwarded as theusedForquery parameter onGET /models/containers. Accepts a single value or a sequence.Checklist:
If a new method has been added it should be referenced in cognite.rst in order to generate docs based on its docstring.