Skip to content
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

ObserverThought reproduce migration script #1338

Merged

Conversation

wintonzheng
Copy link
Contributor

@wintonzheng wintonzheng commented Dec 6, 2024

Important

Add ObserverCruise and ObserverThought entities with new columns and foreign keys to support enhanced artifact tracking.

  • Database Schema:
    • Adds workflow_run_block_id and observer_cruise_id columns to artifacts table in Alembic migration script.
    • Creates observer_cruises and observer_thoughts tables with relevant foreign key constraints in Alembic migration script.
    • Updates foreign key constraints in artifacts table to reference new tables.
  • Models:
    • Adds workflow_run_block_id to ObserverThought class in models.py.
    • Introduces ObserverCruise and ObserverThought classes in models.py with appropriate fields and foreign keys.

This description was created by Ellipsis for 94aebe6. It will automatically update as commits are pushed.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

❌ Changes requested. Reviewed everything up to 2d726bf in 31 seconds

More details
  • Looked at 97 lines of code in 2 files
  • Skipped 0 files when reviewing.
  • Skipped posting 0 drafted comments based on config settings.

Workflow ID: wflow_U4C7Zwo7CiYTCyZL


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.

@@ -73,31 +78,19 @@ def upgrade() -> None:
op.add_column("artifacts", sa.Column("workflow_run_block_id", sa.String(), nullable=True))
op.add_column("artifacts", sa.Column("observer_cruise_id", sa.String(), nullable=True))
op.create_index("org_workflow_run_index", "artifacts", ["organization_id", "workflow_run_id"], unique=False)
op.create_foreign_key(None, "artifacts", "observer_cruises", ["observer_cruise_id"], ["observer_cruise_id"])
Copy link
Contributor

Choose a reason for hiding this comment

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

Using None for foreign key constraint names can make it difficult to identify constraints later. Consider providing explicit names for these constraints.

@wintonzheng wintonzheng force-pushed the shu/add_workflow_run_block_id_to_observer_thought branch from 2d726bf to 94aebe6 Compare December 6, 2024 08:58
@wintonzheng wintonzheng merged commit bb03891 into main Dec 6, 2024
2 checks passed
@wintonzheng wintonzheng deleted the shu/add_workflow_run_block_id_to_observer_thought branch December 6, 2024 08:58
Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

👍 Looks good to me! Incremental review on 94aebe6 in 26 seconds

More details
  • Looked at 58 lines of code in 2 files
  • Skipped 0 files when reviewing.
  • Skipped posting 0 drafted comments based on config settings.

Workflow ID: wflow_ggCCGhLsTB6Ylo6g


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

satansdeer pushed a commit to satansdeer/skyvern that referenced this pull request Dec 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant