Skip to content

Upgrading SQLModel to the latest version#2452

Merged
strickvl merged 36 commits into
developfrom
feature/OSSK-13-upgrading-sqlmodel
Mar 6, 2024
Merged

Upgrading SQLModel to the latest version#2452
strickvl merged 36 commits into
developfrom
feature/OSSK-13-upgrading-sqlmodel

Conversation

@bcdurak

@bcdurak bcdurak commented Feb 20, 2024

Copy link
Copy Markdown
Contributor

Describe changes

I upgraded the version of the SQLModel package that we are using from 0.0.8 to 0.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 Enum values.

For instance, if you are familiar with our component schema (which we defined through SQLModel), we added a field called type which was a StackComponentType:

# the schema of the stack component
class StackComponentSchema(...):
    ...
    type: StackComponentType
    ...
    
# and the stack component type looked like this:
class StackComponentType(StrEnum):
    ...
    ARTIFACT_STORE = "artifact_store"
    ...

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 type had the string artifact_store stored in it as a value. However, with the new changes, SQLModel now gives higher priority to Enum fields and saves the value ARTIFACT_STORE instead. While this is alright if you are starting from scratch, if you have any entry in a table with an Enum field 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 corresponding to_model, update and from_request methods.

@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_utils as 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:

  • I have read the CONTRIBUTING.md document.
  • If my change requires a change to docs, I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • I have based my new branch on develop and the open PR is targeting develop. If your branch wasn't based on develop read Contribution guide on rebasing branch to develop.
  • If my changes require changes to the dashboard, these changes are communicated/requested.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Other (add details above)

@coderabbitai

coderabbitai Bot commented Feb 20, 2024

Copy link
Copy Markdown
Contributor

Important

Auto Review Skipped

Auto reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository.

To trigger a single review, invoke the @coderabbitai review command.

Walkthrough

The updates revolve around dependency version upgrades, the removal of fastapi_utils, and schema modifications across various files. These changes include transitioning from specific enums to string types for attributes in schemas and adjusting methods to accommodate these type changes. Additionally, there are improvements in SQL handling and the removal of unnecessary enum types, streamlining the codebase and reducing dependencies.

Changes

File(s) Summary
pyproject.toml Updated sqlalchemy_utils and sqlmodel versions; removed fastapi_utils.
src/zenml/.../local/local_provider.py Removed fastapi_utils import in check_local_server_dependencies.
src/zenml/.../schemas/artifact_schemas.py
src/zenml/.../schemas/artifact_visualization_schemas.py
src/zenml/.../schemas/component_schemas.py
src/zenml/.../schemas/device_schemas.py
src/zenml/.../schemas/flavor_schemas.py
src/zenml/.../schemas/pipeline_run_schemas.py
src/zenml/.../schemas/run_metadata_schemas.py
src/zenml/.../schemas/secret_schemas.py
src/zenml/.../schemas/step_run_schemas.py
Changed attribute types from enums to str and adjusted methods for handling these types.
src/zenml/.../schemas/tag_schemas.py Added conditional check in update method for "color" field.
src/zenml/.../sql_zen_store.py Simplified SQL handling in filter_and_paginate and migrate_database functions.

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?

Share

Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit-tests for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit tests for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit tests.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • The JSON schema for the configuration file is available here.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/coderabbit-overrides.v2.json

CodeRabbit Discord Community

Join our Discord Community to get help, request features, and share feedback.

@github-actions github-actions Bot added internal To filter out internal PRs and issues enhancement New feature or request labels Feb 20, 2024
@bcdurak bcdurak added the run-slow-ci Tag that is used to trigger the slow-ci label Feb 21, 2024
@bcdurak bcdurak marked this pull request as ready for review February 21, 2024 13:24
@strickvl

Copy link
Copy Markdown
Contributor

@coderabbitai review

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Status

Actionable comments generated: 12

Configuration used: .coderabbit.yaml

Commits Files that changed from the base of the PR and between 1ffe038 and b1b9cbc.
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 type attribute from VisualizationType enum to str in the ArtifactVisualizationSchema class 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_model method, the assignment type=artifact_visualization_request.type.value assumes type is an enum in the request model but assigns it as a string. Confirm that artifact_visualization_request.type is indeed an enum and that this conversion is intended.
src/zenml/zen_stores/schemas/flavor_schemas.py (2)
  • 48-48: The conversion of the type attribute from StackComponentType enum to str in the FlavorSchema class is noted. Ensure that all usages of this attribute throughout the codebase are updated to handle it as a string.
  • 100-101: In the update method, the handling of the type field with value.value assumes value is an enum. Confirm that this conversion is intended and that value is indeed an enum type.
src/zenml/zen_stores/schemas/run_metadata_schemas.py (1)
  • 113-113: The conversion of the type attribute from MetadataTypeEnum to str in the RunMetadataSchema class 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 type attribute from StackComponentType to str in the StackComponentSchema class is noted. Ensure that all usages of this attribute throughout the codebase are updated to handle it as a string.
  • 130-131: In the update method, the handling of the type field with component_update.type.value assumes type is an enum in the update model but assigns it as a string. Confirm that this conversion is intended and that component_update.type is indeed an enum type.
src/zenml/zen_stores/schemas/tag_schemas.py (1)
  • 105-108: In the update method of the TagSchema class, handling the "color" field with value.value assumes value is an enum. Confirm that this conversion is intended and that value is 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 status attribute from OAuthDeviceStatus enum to str in the OAuthDeviceSchema class 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_request method, the assignment status=OAuthDeviceStatus.PENDING.value correctly converts the enum to a string. Ensure that the OAuthDeviceInternalRequest model and its consumers are aware of this change.
  • 156-158: In the update method, converting the status field to OAuthDeviceStatus.LOCKED.value or OAuthDeviceStatus.ACTIVE.value based on the locked flag is a logical approach. However, ensure that this conversion aligns with the expectations of all consumers of the OAuthDeviceSchema.
src/zenml/zen_stores/schemas/secret_schemas.py (3)
  • 57-57: The change from SecretScope enum to str for the scope attribute 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 the scope attribute are correctly handled as strings.
  • 179-179: The assignment scope=secret.scope.value in the from_request method correctly converts the enum to its string value. This is a necessary adjustment following the change of the scope attribute's type to str.
  • 206-207: In the update method, converting the scope value to a string using value.value before assignment ensures compatibility with the attribute's new string type. This maintains consistency in how scope values are handled across the class.
pyproject.toml (2)
  • 63-64: The updates to sqlalchemy_utils and sqlmodel versions in pyproject.toml are 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_utils from 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 on fastapi_utils have 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 ExecutionStatus enum to str for the status attribute 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 the status attribute are correctly handled as strings.
  • 196-196: The assignment status=request.status.value in the from_request method correctly converts the enum to its string value. This is a necessary adjustment following the change of the status attribute's type to str.
  • 303-303: The update to the status attribute in the update method, converting the enum to a string, is correctly implemented to match the attribute's new string type. This ensures consistency in handling status values across the class.
src/zenml/zen_stores/schemas/artifact_schemas.py (1)
  • 166-166: The change from ArtifactType enum to str for the type attribute 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 status field from an enum to a string in StepRunSchema is 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 type fields in StepRunInputArtifactSchema from enums to str aligns with the PR's objective to simplify type handling. This change should make the handling of artifact types more flexible.
  • 341-341: Changing the type fields in StepRunOutputArtifactSchema from enums to str is consistent with the PR's objective and the changes made in StepRunInputArtifactSchema. This ensures uniform handling of artifact types across the schema.

Comment thread src/zenml/zen_stores/schemas/artifact_schemas.py
Comment thread src/zenml/zen_stores/schemas/artifact_schemas.py
Comment thread src/zenml/zen_stores/schemas/step_run_schemas.py
Comment thread src/zenml/zen_stores/schemas/step_run_schemas.py
Comment thread src/zenml/zen_stores/schemas/step_run_schemas.py
Comment thread src/zenml/zen_stores/schemas/artifact_visualization_schemas.py
Comment thread src/zenml/zen_stores/schemas/flavor_schemas.py
Comment thread src/zenml/zen_stores/schemas/run_metadata_schemas.py
Comment thread src/zenml/zen_stores/schemas/component_schemas.py
Comment thread src/zenml/zen_stores/schemas/device_schemas.py

@avishniakov avishniakov left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree with most things @coderabbitai highlighted + test are failing now

@strickvl strickvl removed the run-slow-ci Tag that is used to trigger the slow-ci label Feb 21, 2024
@strickvl strickvl added the run-slow-ci Tag that is used to trigger the slow-ci label Feb 21, 2024
Comment thread src/zenml/integrations/evidently/__init__.py Outdated
Comment thread src/zenml/integrations/airflow/__init__.py Outdated
Comment thread src/zenml/integrations/great_expectations/__init__.py Outdated

@strickvl strickvl left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM aside from the comments I already made.

@bcdurak bcdurak requested a review from strickvl March 1, 2024 13:44
@github-actions

github-actions Bot commented Mar 1, 2024

Copy link
Copy Markdown
Contributor

Quickstart template updates in examples/quickstart have been pushed.

@github-actions

github-actions Bot commented Mar 1, 2024

Copy link
Copy Markdown
Contributor

NLP template updates in examples/e2e_nlp have been pushed.

Comment thread src/zenml/integrations/airflow/__init__.py
Comment thread src/zenml/integrations/evidently/__init__.py
@strickvl strickvl requested a review from avishniakov March 6, 2024 13:24

@avishniakov avishniakov left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🦭

@strickvl strickvl merged commit e93c687 into develop Mar 6, 2024
@strickvl strickvl deleted the feature/OSSK-13-upgrading-sqlmodel branch March 6, 2024 16:17
bcdurak added a commit that referenced this pull request Mar 12, 2024
@bcdurak bcdurak restored the feature/OSSK-13-upgrading-sqlmodel branch March 12, 2024 13:03
bcdurak added a commit that referenced this pull request Mar 15, 2024
* Revert "Upgrading SQLModel to the latest version (#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>
adtygan pushed a commit to adtygan/zenml that referenced this pull request Mar 21, 2024
* 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>
adtygan pushed a commit to adtygan/zenml that referenced this pull request Mar 21, 2024
* 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>
@david101-hunter

Copy link
Copy Markdown

Hi, after I upgrade sqlmodel version, I encounter issue
zenml 0.56.3 requires sqlmodel==0.0.8, but you have sqlmodel 0.0.16 which is incompatible.

I want to intergate feast (0.36.0), however:

zenml 0.56.3 requires pydantic<1.11,>=1.9.0
feast 0.36.0 requires pydantic>=2.0.0, but you have pydantic 1.10.15 which is incompatible

How I can fix that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request internal To filter out internal PRs and issues run-slow-ci Tag that is used to trigger the slow-ci

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants