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

migrate Organization data model from skyvern/forge/sdk/models.py to skyvern/forge/sdk/schemas/organizations.py #1343

Merged
merged 2 commits into from
Dec 7, 2024

Conversation

wintonzheng
Copy link
Contributor

@wintonzheng wintonzheng commented Dec 7, 2024

Important

Migrate Organization model from models.py to schemas/organizations.py and update imports accordingly.

  • Models:
    • Remove Organization and OrganizationAuthToken classes from models.py.
    • Add Organization and OrganizationAuthToken classes to schemas/organizations.py.
  • Imports:
    • Update import paths for Organization from models to schemas/organizations in agent_functions.py, app.py, and cloud_permission_checker.py.
    • Similar updates in db_client.py, checkout.py, and observer.py.
  • Misc:
    • Adjust related imports in agent.py, agent_functions.py, and org_auth_service.py.

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

<!-- ELLIPSIS_HIDDEN -->

> [!IMPORTANT]
> Migrate `Organization` model from `models.py` to `schemas/organizations.py` and update imports accordingly.
>
>   - **Imports**:
>     - Update import paths for `Organization` from `skyvern/forge/sdk/models` to `skyvern/forge/sdk/schemas/organizations` in `agent_functions.py`, `app.py`, and `cloud_permission_checker.py`.
>     - Similar updates in `db_client.py`, `checkout.py`, and `observer.py`.
>   - **Models**:
>     - Remove `Organization` and `OrganizationAuthToken` classes from `models.py`.
>     - Add `Organization` and `OrganizationAuthToken` classes to `schemas/organizations.py`.
>   - **Misc**:
>     - Adjust related imports in `agent.py`, `agent_functions.py`, and `org_auth_service.py`.
>
> <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 ea501c5324f88a81c7abf3a1ea5f44ae8d821fa9. 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 cc68b2c in 28 seconds

More details
  • Looked at 253 lines of code in 13 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. skyvern/forge/sdk/services/org_auth_service.py:16
  • Draft comment:
    Organize imports according to PEP 8 guidelines. Group standard library imports, third-party imports, and local imports separately with a blank line between each group. This applies to other files as well.
  • Reason this comment was not posted:
    Confidence changes required: 33%
    The migration of the Organization model from models.py to schemas/organizations.py seems to be done correctly. However, there is a minor issue with the import statements in some files. The import statements should be organized according to PEP 8 guidelines, which suggest grouping imports into standard library imports, related third-party imports, and local application/library-specific imports, with a blank line between each group.

Workflow ID: wflow_7UFekTWQSoE2FusX


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 cc68b2c in 45 seconds

More details
  • Looked at 253 lines of code in 13 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. skyvern/forge/sdk/services/org_auth_service.py:16
  • Draft comment:
from skyvern.forge.sdk.db.enums import OrganizationAuthTokenType
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable:
    The comment suggests an import path that is not supported by the diff. The diff shows OrganizationAuthTokenType being imported from schemas, not db.enums. There is no evidence in the diff that supports the suggested change to db.enums.
    I might be missing some context about where OrganizationAuthTokenType is defined, but based on the diff, it seems to be correctly imported from schemas.
    The diff provides clear evidence that the import path was changed to schemas, and there is no indication that db.enums is the correct path.
    Delete the comment as it suggests an incorrect import path change that is not supported by the diff.

Workflow ID: wflow_96GtKQSHVh7rRtJV


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 34734d6 in 22 seconds

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

Workflow ID: wflow_2PeXjCb434d0cDUS


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

@wintonzheng wintonzheng merged commit 7591873 into main Dec 7, 2024
2 checks passed
@wintonzheng wintonzheng deleted the shu/artifacts_for_observers branch December 7, 2024 01:15
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