-
Notifications
You must be signed in to change notification settings - Fork 16.4k
Split SDK and serialized asset classes #58993
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
86783ba to
ffbc597
Compare
1f8b27f to
d6a2e34
Compare
00ca7f1 to
3582606
Compare
I also took the oppertunity to improve asset timetable tests quite a bit.
This needs to eventually be migrated to SDK too. I don't know how yet...
3582606 to
e7d757e
Compare
potiuk
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good conceptually. Similar to what we've done in the past AIP-44 implementation, I am ok with merging it, just have two small questions mostly for educational purpose.
kaxil
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#protm
Next for #52141.
All asset-related classes now have a counterpart in
airflow.serializatio.definitions.assetsprefixed with Serialized. Many tweaks in Core to use those types instead.While working on this, I realised the partition work also currently breaks the SDK-Core boundary. (Specifically PartitionMapper.) We need to figure out a way to do this correctly… cc @Lee-W @dstandish