Drop all the foreign keys on artifacts table except for the organization_id #1352
Conversation
"<!-- ELLIPSIS_HIDDEN --> ---- > [!IMPORTANT] > Add support for observer artifact creation in `ArtifactManager` with new schema models and database updates. > > - **Artifact Manager**: > - Adds `create_observer_thought_artifact()` in `manager.py` for creating observer thought artifacts. > - Introduces `_create_artifact()` as a helper method in `manager.py`. > - **Schemas**: > - Adds `ObserverCruise` and `ObserverThought` models in `schemas/observers.py`. > - Defines `ObserverCruiseStatus` enum for cruise status. > - **Database Models**: > - Renames `ObserverCruise` to `ObserverCruiseModel` and `ObserverThought` to `ObserverThoughtModel` in `models.py`. > - **Storage**: > - Adds `build_observer_thought_uri()` and `build_observer_cruise_uri()` in `base.py`, `local.py`, and `s3.py` for URI construction. > - **Misc**: > - Updates `convert_to_artifact()` in `utils.py` to handle new fields. > > <sup>This description was created by </sup>[<img alt="Ellipsis" src="https://img.shields.io/badge/Ellipsis-blue?color=175173">](https://www.ellipsis.dev?ref=Skyvern-AI%2Fskyvern-cloud&utm_source=github&utm_medium=referral)<sup> for 703d4a6c7efe742d40e7bea91cba13b04fcfe558. It will automatically update as commits are pushed.</sup> ---- <!-- ELLIPSIS_HIDDEN -->\n"
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
There was a problem hiding this comment.
❌ Changes requested. Incremental review on 7624b8d in 26 seconds
More details
- Looked at
24lines of code in1files - Skipped
0files when reviewing. - Skipped posting
0drafted comments based on config settings.
Workflow ID: wflow_I1j66rlcXDcqDoFH
Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
| workflow_run_block_id = Column(String, ForeignKey("workflow_run_blocks.workflow_run_block_id")) | ||
| observer_cruise_id = Column(String, ForeignKey("observer_cruises.observer_cruise_id")) | ||
| observer_thought_id = Column(String, ForeignKey("observer_thoughts.observer_thought_id")) | ||
| workflow_run_id = Column(String) |
There was a problem hiding this comment.
Removing ForeignKey constraints for workflow_run_id, workflow_run_block_id, observer_cruise_id, and observer_thought_id in ArtifactModel can lead to data integrity issues. Consider keeping these constraints to ensure referential integrity.
There was a problem hiding this comment.
❌ Changes requested. Reviewed everything up to 7624b8d in 29 seconds
More details
- Looked at
24lines of code in1files - Skipped
0files when reviewing. - Skipped posting
0drafted comments based on config settings.
Workflow ID: wflow_YDMJ1oJvKlUDeFd2
Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
| workflow_run_block_id = Column(String, ForeignKey("workflow_run_blocks.workflow_run_block_id")) | ||
| observer_cruise_id = Column(String, ForeignKey("observer_cruises.observer_cruise_id")) | ||
| observer_thought_id = Column(String, ForeignKey("observer_thoughts.observer_thought_id")) | ||
| workflow_run_id = Column(String) |
There was a problem hiding this comment.
Removing foreign key constraints from workflow_run_id, workflow_run_block_id, observer_cruise_id, and observer_thought_id may lead to data integrity issues. Consider keeping these constraints if these fields are meant to reference existing records.
wintonzheng
changed the title
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on 848e112 in 7 seconds
More details
- Looked at
63lines of code in1files - Skipped
0files when reviewing. - Skipped posting
1drafted comments based on config settings.
1. alembic/versions/2024_12_08_0131-8069e38dc1b4_drop_all_the_foreign_keys_on_artifacts_.py:1
- Draft comment:
Typo in docstring: 'orgnaization_id' should be 'organization_id'. - Reason this comment was not posted:
Confidence changes required:50%
The PR description mentions dropping foreign keys except for organization_id, but the docstring in the migration script has a typo in 'organization_id'.
Workflow ID: wflow_Q364kwexdMHIr9AK
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
"
Important
Remove foreign keys from
artifactstable exceptorganization_idand updateArtifactModelaccordingly.2024_12_08_0131-8069e38dc1b4_drop_all_the_foreign_keys_on_artifacts_.py, drop foreign keys onartifactstable exceptorganization_id.org_observer_cruise_indexindex onartifactstable.models.py, removeForeignKeyconstraints fromworkflow_run_id,workflow_run_block_id,observer_cruise_id, andobserver_thought_idinArtifactModel.org_observer_cruise_indexindex toArtifactModel.This description was created by
for 848e112. It will automatically update as commits are pushed.
\n"