Upgrading SQLModel to the latest version#2452
Conversation
|
Important Auto Review SkippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the To trigger a single review, invoke the WalkthroughThe updates revolve around dependency version upgrades, the removal of Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
|
@coderabbitai review |
There was a problem hiding this comment.
Review Status
Actionable comments generated: 12
Configuration used: .coderabbit.yaml
Files selected for processing (13)
- pyproject.toml (2 hunks)
- src/zenml/zen_server/deploy/local/local_provider.py (1 hunks)
- src/zenml/zen_stores/schemas/artifact_schemas.py (3 hunks)
- src/zenml/zen_stores/schemas/artifact_visualization_schemas.py (3 hunks)
- src/zenml/zen_stores/schemas/component_schemas.py (3 hunks)
- src/zenml/zen_stores/schemas/device_schemas.py (4 hunks)
- src/zenml/zen_stores/schemas/flavor_schemas.py (3 hunks)
- src/zenml/zen_stores/schemas/pipeline_run_schemas.py (5 hunks)
- src/zenml/zen_stores/schemas/run_metadata_schemas.py (2 hunks)
- src/zenml/zen_stores/schemas/secret_schemas.py (4 hunks)
- src/zenml/zen_stores/schemas/step_run_schemas.py (7 hunks)
- src/zenml/zen_stores/schemas/tag_schemas.py (1 hunks)
- src/zenml/zen_stores/sql_zen_store.py (2 hunks)
Files not reviewed due to errors (1)
- (no review received)
Files skipped from review due to trivial changes (1)
- src/zenml/zen_server/deploy/local/local_provider.py
Additional comments: 23
src/zenml/zen_stores/schemas/artifact_visualization_schemas.py (2)
- 39-39: The conversion of the
typeattribute fromVisualizationTypeenum tostrin theArtifactVisualizationSchemaclass is noted. Ensure that all usages of this attribute throughout the codebase are updated to handle it as a string.- 73-73: In the
from_modelmethod, the assignmenttype=artifact_visualization_request.type.valueassumestypeis an enum in the request model but assigns it as a string. Confirm thatartifact_visualization_request.typeis indeed an enum and that this conversion is intended.src/zenml/zen_stores/schemas/flavor_schemas.py (2)
- 48-48: The conversion of the
typeattribute fromStackComponentTypeenum tostrin theFlavorSchemaclass is noted. Ensure that all usages of this attribute throughout the codebase are updated to handle it as a string.- 100-101: In the
updatemethod, the handling of thetypefield withvalue.valueassumesvalueis an enum. Confirm that this conversion is intended and thatvalueis indeed an enum type.src/zenml/zen_stores/schemas/run_metadata_schemas.py (1)
- 113-113: The conversion of the
typeattribute fromMetadataTypeEnumtostrin theRunMetadataSchemaclass is noted. Ensure that all usages of this attribute throughout the codebase are updated to handle it as a string.src/zenml/zen_stores/schemas/component_schemas.py (2)
- 52-52: The conversion of the
typeattribute fromStackComponentTypetostrin theStackComponentSchemaclass is noted. Ensure that all usages of this attribute throughout the codebase are updated to handle it as a string.- 130-131: In the
updatemethod, the handling of thetypefield withcomponent_update.type.valueassumestypeis an enum in the update model but assigns it as a string. Confirm that this conversion is intended and thatcomponent_update.typeis indeed an enum type.src/zenml/zen_stores/schemas/tag_schemas.py (1)
- 105-108: In the
updatemethod of theTagSchemaclass, handling the "color" field withvalue.valueassumesvalueis an enum. Confirm that this conversion is intended and thatvalueis indeed an enum type. This change might require all callers to pass enum types for the "color" field.src/zenml/zen_stores/schemas/device_schemas.py (3)
- 47-47: The conversion of the
statusattribute fromOAuthDeviceStatusenum tostrin theOAuthDeviceSchemaclass is noted. Ensure that all usages of this attribute throughout the codebase are updated to handle it as a string.- 124-124: In the
from_requestmethod, the assignmentstatus=OAuthDeviceStatus.PENDING.valuecorrectly converts the enum to a string. Ensure that theOAuthDeviceInternalRequestmodel and its consumers are aware of this change.- 156-158: In the
updatemethod, converting thestatusfield toOAuthDeviceStatus.LOCKED.valueorOAuthDeviceStatus.ACTIVE.valuebased on thelockedflag is a logical approach. However, ensure that this conversion aligns with the expectations of all consumers of theOAuthDeviceSchema.src/zenml/zen_stores/schemas/secret_schemas.py (3)
- 57-57: The change from
SecretScopeenum tostrfor thescopeattribute aligns with the PR's objective to handle enum types as strings due to dependency updates. This change should be cross-verified in the application to ensure that all interactions with thescopeattribute are correctly handled as strings.- 179-179: The assignment
scope=secret.scope.valuein thefrom_requestmethod correctly converts the enum to its string value. This is a necessary adjustment following the change of thescopeattribute's type tostr.- 206-207: In the
updatemethod, converting thescopevalue to a string usingvalue.valuebefore assignment ensures compatibility with the attribute's new string type. This maintains consistency in howscopevalues are handled across the class.pyproject.toml (2)
- 63-64: The updates to
sqlalchemy_utilsandsqlmodelversions inpyproject.tomlare in line with the PR's objectives to upgrade these dependencies to newer versions. Ensure that the rest of the application is compatible with these new versions, especially regarding any breaking changes or deprecations introduced between the old and new versions.- 63-64: The removal of
fastapi_utilsfrom the dependencies is not directly visible in the provided code snippet. However, based on the PR's objectives, this simplification is expected to reduce the complexity of the project's dependency tree. Verify that all functionalities previously relying onfastapi_utilshave been appropriately refactored or that the dependency was indeed unused.src/zenml/zen_stores/schemas/pipeline_run_schemas.py (3)
- 70-70: The change from
ExecutionStatusenum tostrfor thestatusattribute aligns with the PR's objective to handle enum types as strings due to dependency updates. This change should be cross-verified in the application to ensure that all interactions with thestatusattribute are correctly handled as strings.- 196-196: The assignment
status=request.status.valuein thefrom_requestmethod correctly converts the enum to its string value. This is a necessary adjustment following the change of thestatusattribute's type tostr.- 303-303: The update to the
statusattribute in theupdatemethod, converting the enum to a string, is correctly implemented to match the attribute's new string type. This ensures consistency in handlingstatusvalues across the class.src/zenml/zen_stores/schemas/artifact_schemas.py (1)
- 166-166: The change from
ArtifactTypeenum tostrfor thetypeattribute aligns with the PR's objective to handle dependency updates. This modification simplifies the handling of artifact types by using string representations.src/zenml/zen_stores/schemas/step_run_schemas.py (3)
- 60-60: Changing the
statusfield from an enum to a string inStepRunSchemais consistent with the PR's objective to simplify type handling by using strings. This change should make the handling of execution status more flexible.- 305-305: Changing the
typefields inStepRunInputArtifactSchemafrom enums tostraligns with the PR's objective to simplify type handling. This change should make the handling of artifact types more flexible.- 341-341: Changing the
typefields inStepRunOutputArtifactSchemafrom enums tostris consistent with the PR's objective and the changes made inStepRunInputArtifactSchema. This ensures uniform handling of artifact types across the schema.
avishniakov
left a comment
There was a problem hiding this comment.
Agree with most things @coderabbitai highlighted + test are failing now
strickvl
left a comment
There was a problem hiding this comment.
LGTM aside from the comments I already made.
|
Quickstart template updates in |
|
NLP template updates in |
* upgrade sqlmodel, remove fastapi-utils * remove fastapi-utils as a requirement * small query bug * new migration scriptqq * adding some comments * fixing docstring * change in schemas instead of migration * fixing the tag updates * formatting * latest sqlmodel * fixing versions * formatting and linting * new version for ge and formatting and linting * new versions * zen store fix * fixing the spaces * checking * sorting requirements * half half install hack * remove bc * fixing the sql model bugs * added an extra comment on dependencies * formatting examples * Auto-update of Starter template * Auto-update of NLP template --------- Co-authored-by: Alex Strick van Linschoten <strickvl@users.noreply.github.com> Co-authored-by: Alex Strick van Linschoten <stricksubscriptions@fastmail.fm> Co-authored-by: GitHub Actions <actions@github.com>
* Revert "Upgrading SQLModel to the latest version (zenml-io#2452)" This reverts commit e93c687. * Auto-update of Starter template --------- Co-authored-by: GitHub Actions <actions@github.com> Co-authored-by: Alex Strick van Linschoten <strickvl@users.noreply.github.com>
|
Hi, after I upgrade sqlmodel version, I encounter issue I want to intergate feast (0.36.0), however:
How I can fix that. |
Describe changes
I upgraded the version of the SQLModel package that we are using from
0.0.8to0.0.16.The most critical factor in this upgrade stems from this PR right here. With this change, they have changed the way that they handle
Enumvalues.For instance, if you are familiar with our component schema (which we defined through SQLModel), we added a field called
typewhich was aStackComponentType:With this setup, when we registered, for instance, a new artifact store, we created an entry in the components table of our DB where the column
typehad the stringartifact_storestored in it as a value. However, with the new changes, SQLModel now gives higher priority toEnumfields and saves the valueARTIFACT_STOREinstead. While this is alright if you are starting from scratch, if you have any entry in a table with anEnumfield zenml will fail after the upgrade. Instead of taking the migration route, we decided to adjust our schemas to use str fields instead and updated the correspondingto_model,updateandfrom_requestmethods.@avishniakov every enum field is using a similar approach to the one that you took with the
TagSchemas and their color field.Additionally, I have removed
fastapi_utilsas a dependency as it was completely unused and tweaked the code a bit to work with the new SQLModel version.Pre-requisites
Please ensure you have done the following:
developand the open PR is targetingdevelop. If your branch wasn't based on develop read Contribution guide on rebasing branch to develop.Types of changes