feat(transformations): add OneLake external data source data classes#2714
feat(transformations): add OneLake external data source data classes#2714Khyat-Cognite wants to merge 23 commits into
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2714 +/- ##
==========================================
- Coverage 93.70% 93.68% -0.03%
==========================================
Files 504 506 +2
Lines 51120 51368 +248
==========================================
+ Hits 47904 48125 +221
- Misses 3216 3243 +27
🚀 New features to boost your workflow:
|
Add read/write/list/usability data classes for Fabric OneLake external data sources, public re-exports, IAM capability helper, and unit tests for serialization round-trips.
The external data source exports accidentally dropped MeteringData from the public data_classes surface and added an unrelated RecordId export.
Replace the date-expired authlib/cryptography shim with a Pyodide version check so stlite's 0.26.2 job keeps working.
98e71c4 to
ad1e459
Compare
There was a problem hiding this comment.
Code Review
This pull request introduces support for Fabric OneLake external data sources in transformations, adding new data classes, ACL capabilities, and corresponding unit tests. The feedback focuses on reducing boilerplate and aligning with the style guide by removing redundant custom dump methods on several data classes, simplifying others using super().dump(), and removing manual load/dump unit tests that are already covered automatically by the SDK's base test suite.
Drop redundant custom dump implementations on simple OneLake credential and usability classes, use super().dump() on source models, and remove manual load/dump unit tests covered by test_base.py per style guide.
|
All gemini fixes were applied -- it was right lot of redundancy. |
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request adds support for Fabric OneLake external data sources in transformations, including new data classes, capabilities, and unit tests. It also refactors a temporary Pyodide workaround in test-pyodide.js to use a version-based check. The feedback suggests using parseInt instead of Number when parsing Pyodide version components to robustly handle pre-release or development versions containing non-numeric characters.
…arse *Write classes are CDF upsert models, not lakehouse write APIs. Use parseInt for Pyodide version components per Gemini review.
3243639 to
66dcea8
Compare
|
/gemini review |
karthikeyanraghavan
left a comment
There was a problem hiding this comment.
LGTM. This PR contains only new data models for zero copy enhancements to transformations.
Rename OneLakeCredentialsRead and OneLakeDataSourceSettingsRead to match SDK convention where read models have no suffix and write models use Write. Co-authored-by: Cursor <cursoragent@cursor.com>
haakonvt
left a comment
There was a problem hiding this comment.
Did an initial pass, but did not make it all the way through. You can save a lot of walruses by checking out the load_if method.
Co-authored-by: Håkon V. Treider <haakonvt@gmail.com>
Co-authored-by: Håkon V. Treider <haakonvt@gmail.com>
Co-authored-by: Håkon V. Treider <haakonvt@gmail.com>
…odels Use _load_if, tighten client_secret handling, redact str/repr, and require client_secret when converting read credentials via as_write(). Co-authored-by: Cursor <cursoragent@cursor.com>
Add Returns sections and single-line Args formatting expected by pydoclint and the custom docstring checker. Co-authored-by: Cursor <cursoragent@cursor.com>
ExternalDataSource.as_write requires client_secret when the read model includes credentials; update generic writable tests accordingly. Co-authored-by: Cursor <cursoragent@cursor.com>
Use isinstance narrowing so client_secret is only passed on the concrete ExternalDataSource types, not WriteableCogniteResource. Co-authored-by: Cursor <cursoragent@cursor.com>
haakonvt
left a comment
There was a problem hiding this comment.
A few more comments, mostly lgtm, but let's revisit the onelake classmethod (feel free to bring the discussion to slack!)
Add OneLakeExternalDataSource(Write) subclasses, export from transformations package only, register ACL in capability tests, and trim redundant dump tests per SDK conventions. Co-authored-by: Cursor <cursoragent@cursor.com>
Single import block with noqa F401; avoids ruff isort splitting as-aliases. Co-authored-by: Cursor <cursoragent@cursor.com>
Return UnknownExternalDataSource for unsupported formats, require format on load per jetfire contract, and flatten ExternalDataSourceCore away. Co-authored-by: Cursor <cursoragent@cursor.com>
ExternalDataSource is now format-agnostic; settings and as_write() live on OneLakeExternalDataSource. UnknownExternalDataSource has no typed settings. Write base drops settings; OneLakeExternalDataSourceWrite owns format-specific dump/upsert shape. Co-authored-by: Cursor <cursoragent@cursor.com>
|
Ahh... Will fix lint |
Annotate optional settings on OneLakeExternalDataSourceWrite, cast _load dispatch return, and narrow test_base list/as_write helpers for mypy. Co-authored-by: Cursor <cursoragent@cursor.com>
@haakonvt final final round? |
Use explicit base return type instead of Self when delegating to format subclasses, satisfying mypy on polymorphic _load.
…eturn as_write always raises ValueError for unknown formats; NoReturn makes the contract explicit to mypy.
Make ExternalDataSource an ABC so test_base skips it. Fold partial settings into OneLakeExternalDataSourceWrite.__init__ and remove the __new__/with_settings factory. Revert unrelated FileMetadataList loop change; mypy ignore documents the list-item union only.
Drop flat credential/location kwargs from __init__; callers pass OneLakeDataSourceSettingsWrite, matching the read model and API shape.
CDF-28228
Summary
Slice 1 of 3 — splits PR #2713 into reviewable pieces.
This PR adds the data model layer for Fabric OneLake external data sources used by transformations. No client API or documentation yet.
What's in this PR
Production code
cognite/client/data_classes/transformations/external_data.py— read/write/list/usability models, OneLake credentials/settings,ExternalDataSourceWrite.onelake()factorycognite/client/data_classes/__init__.py— public re-exports (ExternalDataSource,ExternalDataSourceWrite,OneLake*, etc.)cognite/client/data_classes/capabilities.py—TransformationsExternalDataSourcesAcl(READ / WRITE / USE)Tests (co-located)
tests/tests_unit/test_data_classes/test_transformations/test_external_data.py— serialization round-trips,onelake()factory, dump/loadBackend contract (jetfire)
Registers credentials for transform SQL:
ext_onelake('external_id', 'table'[, 'schema'])/transformations/external_data/transformations/external_data/delete/transformations/external_data/usabilityWhat comes next
feat/onelake-edc-apiclient.transformations.external_data_sources(list, upsert, delete, verify_usability), sync codegen, mocks, API unit + integration testsfeat/onelake-edc-docstransformations_external_data.rst, toctree linkMerge this PR first, then review PR 2 (stacked on this branch).
Test plan
pytest tests/tests_unit/test_data_classes/test_transformations/test_external_data.pylint/test_coreCI greenParent
Replaces the data-model portion of #2713. Close #2713 after all three slices are merged to
master.