feat(Data Modeling, Files): Add files api to data modeling with /retrieve and /list#2647
feat(Data Modeling, Files): Add files api to data modeling with /retrieve and /list#2647haakonvt wants to merge 11 commits into
Conversation
| self, | ||
| nodes: NodeId | tuple[str, str], | ||
| *, | ||
| source: View | ViewId | tuple[str, str, str] = COGNITE_FILE_VIEW_ID, |
There was a problem hiding this comment.
Note for reviewer: ViewIds are immutable so they are fine to use as default args
![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
This pull request introduces the DataModelingFilesAPI (and its auto-generated sync counterpart) to support retrieving and listing file nodes within the data modeling framework. It also adds a drop_source method to Node instances to remove a source and its properties, restoring quick property access when a single source remains. Feedback on the changes highlights a bug in drop_source where dropping the last remaining source leaves a stale property lookup reference, a style guide violation where assert_never is used on user-supplied input instead of raising a descriptive TypeError, and a request to add a test assertion to verify that quick property access is disabled when no sources remain.
haakonvt
force-pushed
the
add-files-api-to-data-modeling
branch
2 times, most recently
from
524fc7c to
9328c32
Compare
haakonvt
force-pushed
the
add-files-api-to-data-modeling
branch
from
9328c32 to
a09f5dd
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #2647 +/- ##
==========================================
+ Coverage 93.05% 93.08% +0.02%
==========================================
Files 486 489 +3
Lines 49671 49879 +208
==========================================
+ Hits 46221 46428 +207
- Misses 3450 3451 +1
🚀 New features to boost your workflow:
|
| # User has passed a custom source, we include CogniteFile source to guarantee only file nodes | ||
| # are returned. We will later strip them (hence the 'True' flag) to avoid returning nodes with | ||
| # properties from multiple sources as they are very annoying to work with in the SDK. | ||
| return [source_as_id, COGNITE_FILE_VIEW_ID], True |
There was a problem hiding this comment.
nit: can we document the return tuple type? hard to say what this True bool means here..
There was a problem hiding this comment.
What more do you need 😂
| sources, strip = _resolve_source(source) | ||
| result = await self._cognite_client.data_modeling.instances.retrieve_nodes(nodes=nodes, sources=sources) # type: ignore[arg-type] |
There was a problem hiding this comment.
can _resolve_source() return Sequence[ViewId] so we get rid of type ignore?
There was a problem hiding this comment.
I’ll widen what retrieve_nodes accept 👍
| ) | ||
| if strip: | ||
| for node in results: | ||
| node.drop_source(COGNITE_FILE_VIEW_ID) |
There was a problem hiding this comment.
Is strip a bad word? 😆
2 participants
This is the first of 3 or 4 parts, adding a "Data Modeling specific Files API interface" to the SDK that should significantly reduce the current friction.
In this part 1 I've added:
filesAPI to toclient.data_modeling