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-33493: Add serialization support for datastore records #677
Conversation
Codecov Report
@@ Coverage Diff @@
## main #677 +/- ##
==========================================
- Coverage 84.32% 84.29% -0.03%
==========================================
Files 242 243 +1
Lines 31055 31089 +34
Branches 5219 5235 +16
==========================================
+ Hits 26187 26208 +21
- Misses 3706 3714 +8
- Partials 1162 1167 +5
Continue to review full report at Codecov.
|
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! I have some minor comments that I think should be addressed, but the big picture looks great.
Adds a bunch of pydantic classes for datastore records and implements serialization of the records in `Quantum` class. I switched back `QuantumProvenanceData` to be a pydantic model to use the same pydantic classes for datastore records. Moved `DatastoreRecordData` class (and its pydantic friend) to a separate module, for some reason pydantic was not happy when I added `SerializedDatastoreRecordData` class to the `datastore` module.
Dropped `dataset_ids` member in favor of `records` keys. Dropped support for strings vs. UUIDs in a regular constructor, strings are only supported in direct() method. QuantumProvenanceData class had to add direct() method to enable construction from JSON data.
70d1a35
to
9dcbcbb
Compare
Adds a bunch of pydantic classes for datastore records and implements
serialization of the records in
Quantum
class. I switched backQuantumProvenanceData
to be a pydantic model to use the same pydanticclasses for datastore records. Moved
DatastoreRecordData
class (and itspydantic friend) to a separate module, for some reason pydantic was not
happy when I added
SerializedDatastoreRecordData
class to thedatastore
module.Checklist
doc/changes