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

Add org index to the actions table #1673

Merged
merged 2 commits into from
Jan 29, 2025
Merged

Add org index to the actions table #1673

merged 2 commits into from
Jan 29, 2025

Conversation

wintonzheng
Copy link
Contributor

@wintonzheng wintonzheng commented Jan 29, 2025

Important

Add action_org_created_at_index to actions table for optimized querying by organization_id and created_at.

  • Database Migration:
    • Adds Alembic migration 2025_01_29_0738-df80b5d155d0_add_action_org_created_at_index.py to create action_org_created_at_index on actions table for organization_id and created_at.
    • Includes upgrade() and downgrade() functions to add and remove the index, respectively.
  • Model Changes:
    • Updates ActionModel in models.py to include action_org_created_at_index for organization_id and created_at using desc().

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

<!-- ELLIPSIS_HIDDEN -->

> [!IMPORTANT]
> Add `action_org_created_at_index` to `actions` table for optimized querying by `organization_id` and `created_at`.
>
>   - **Database Migration**:
>     - Adds Alembic migration `2025_01_29_0723-43edb82cf7a9_add_action_org_created_at_index.py` to create `action_org_created_at_index` on `actions` table.
>     - Index is created concurrently with a 1-hour statement timeout.
>   - **Model Changes**:
>     - Updates `ActionModel` in `models.py` to include `action_org_created_at_index` on `organization_id` and `created_at` (descending).
>
> <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 d69dd71f9574507efe8e5a396497bf81ae3cc7f5. It will automatically update as commits are pushed.</sup>

<!-- ELLIPSIS_HIDDEN -->
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 9c548dd in 9 seconds

More details
  • Looked at 24 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. skyvern/forge/sdk/db/models.py:478
  • Draft comment:
    The PR description is empty. It's best practice to include a description explaining the reason for the change, especially when adding a new index.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The PR adds a new index to the ActionModel table, which is a valid change. However, the PR description is empty, which is not a best practice as it should explain the reason for the change.

Workflow ID: wflow_gvgRkMZGtso8bTde


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

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! Reviewed everything up to 9c548dd in 12 seconds

More details
  • Looked at 24 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. skyvern/forge/sdk/db/models.py:477
  • Draft comment:
    The PR description is empty. It's best practice to include a description explaining the intent and context of the change.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The PR adds a new index to the ActionModel table, which is a valid change. However, the PR description is empty, which is not a best practice. A description should be provided to explain the intent and context of the change.

Workflow ID: wflow_eM7IOdyDSuy2V3ZO


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

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 dd3142b in 9 seconds

More details
  • Looked at 39 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. alembic/versions/2025_01_29_0738-df80b5d155d0_add_action_org_created_at_index.py:25
  • Draft comment:
    Using sa.text("created_at DESC") is unconventional for specifying index order. Consider using sa.desc("created_at") instead for clarity and consistency with SQLAlchemy's API.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The PR adds an index to the 'actions' table, which is a good practice for performance improvement. However, the use of 'sa.text' for 'created_at DESC' is unconventional and might not be necessary.

Workflow ID: wflow_CN8ug9l8KanefWjf


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

@wintonzheng wintonzheng merged commit d1e0a17 into main Jan 29, 2025
7 checks passed
@wintonzheng wintonzheng deleted the suchintan.add-index branch January 29, 2025 07:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant