-
Notifications
You must be signed in to change notification settings - Fork 14
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
DM-41114: Add dimension record containers #911
Closed
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
TallJimbo
force-pushed
the
tickets/DM-41114
branch
from
November 27, 2023 20:26
cd1d990
to
607a552
Compare
Somehow pydantic 1.10 is being installed. |
TallJimbo
force-pushed
the
tickets/DM-41114
branch
from
December 11, 2023 21:41
607a552
to
7ae2032
Compare
TallJimbo
force-pushed
the
tickets/DM-41114
branch
from
December 17, 2023 13:50
7ae2032
to
c5eae37
Compare
This is the first small step in moving caching out of the registry's DimensionRecordStorage classes.
TallJimbo
force-pushed
the
tickets/DM-41114
branch
from
December 17, 2023 20:00
c5eae37
to
6ae6df1
Compare
When we convert the dimensions.yaml config into a DimensionUniverse, we've been converting the rather limited set of types supported in the configuration into just a few of the very specific types used by SQLAlchemy (in the ddl.FieldSpec objects), and that makes it hard to write a mapping from the Python form of that information to some other schema system (like Pydantic or Arrow). The new ColumnSpce objects map more directly to what's in the config (they're Pydantic models; this is one tiny step towards making dimensions.yaml Pydantic-validated), and while right now they live alongside the ddl objects everywhere, eventually I'd like to replace the ddl stuff entirely with direct usage of SQLAlchemy Table and Column objects. Fully removing the ddl stuff will require an RFC as well, since they leak into the public interface a bit via RecordClass.fields.
TallJimbo
force-pushed
the
tickets/DM-41114
branch
from
December 17, 2023 21:07
6ae6df1
to
cb26583
Compare
It should be safe to revert this commit once we require Pydantic v2.
TallJimbo
force-pushed
the
tickets/DM-41114
branch
from
December 17, 2023 21:16
cb26583
to
1dd8413
Compare
Closing this draft in favor of #928 . |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Checklist
doc/changes