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 aiohttp helper from cloud to open source #572

Merged
merged 1 commit into from
Jul 10, 2024
Merged

Conversation

ykeremy
Copy link
Contributor

@ykeremy ykeremy commented Jul 10, 2024

🚀 This description was created by Ellipsis for commit cb90067

Summary:

Migrated aiohttp_helper module and HttpException class from cloud to skyvern package, updating import paths accordingly.

Key points:

  • Migrated aiohttp_helper module from cloud to skyvern package.
  • Updated import paths for aiohttp_get_json in cloud/core/authentication.py, cloud/core/mihomo/mihomo.py, cloud/webeye/proxy.py, and scripts/luca/walmart_client.py.
  • Moved aiohttp_helper.py from cloud/core/aiohttp_helper.py to skyvern/forge/sdk/core/aiohttp_helper.py.
  • Migrated HttpException class from cloud/exceptions.py to skyvern/exceptions.py.
  • Removed HttpException class from cloud/exceptions.py and added it to skyvern/exceptions.py.

Generated with ❤️ by ellipsis.dev

<!-- ELLIPSIS_HIDDEN -->

| 🚀 | This description was created by [Ellipsis](https://www.ellipsis.dev) for commit 3323cc377c127ab276c06f2372d0809de38d1c93  |
|--------|--------|

### Summary:
Migrated `aiohttp_helper` module and `HttpException` class from `cloud` to `skyvern` package, updating import paths accordingly.

**Key points**:
- Migrated `aiohttp_helper` from `cloud` to `skyvern` package.
- Updated import paths for `aiohttp_get_json` in `cloud/core/authentication.py`, `cloud/core/mihomo/mihomo.py`, `cloud/webeye/proxy.py`, and `scripts/luca/walmart_client.py`.
- Moved `aiohttp_helper.py` from `cloud/core/aiohttp_helper.py` to `skyvern/forge/sdk/core/aiohttp_helper.py`.
- Moved `HttpException` class from `cloud/exceptions.py` to `skyvern/exceptions.py`.
- Removed `HttpException` class from `cloud/exceptions.py` and added it to `skyvern/exceptions.py`.

----
Generated with ❤️ by [ellipsis.dev](https://www.ellipsis.dev)

<!-- ELLIPSIS_HIDDEN -->
@ykeremy ykeremy added the sync label Jul 10, 2024
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 cb90067 in 31 seconds

More details
  • Looked at 65 lines of code in 2 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. skyvern/forge/sdk/core/aiohttp_helper.py:38
  • Draft comment:
    The usage of HttpException here is consistent with its definition in skyvern.exceptions. The exception handling and retry logic are well-implemented.
  • Reason this comment was not posted:
    Confidence changes required: 0%
    The HttpException class is defined in skyvern.exceptions and is imported correctly in aiohttp_helper.py. The usage of HttpException in line 38 is consistent with its definition, which includes a status code and URL, and optionally a message. The function aiohttp_get_json is designed to make HTTP GET requests and handle exceptions appropriately, including retries and logging. The code structure and logic appear sound, with proper exception handling and use of asynchronous programming with aiohttp.

Workflow ID: wflow_PPSmBsUOkMoyXn3s


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.

❌ Changes requested. Reviewed everything up to cb90067 in 48 seconds

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

Workflow ID: wflow_UgeuJtoxZhhOiiYZ


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.

import aiohttp
import structlog

from skyvern.exceptions import HttpException
Copy link
Contributor

Choose a reason for hiding this comment

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

The import statement for HttpException is incorrect. It should be SkyvernHTTPException based on the existing class definitions in skyvern.exceptions. Please update the import and usage accordingly.

Suggested change
from skyvern.exceptions import HttpException
from skyvern.exceptions import SkyvernHTTPException as HttpException

@wintonzheng wintonzheng merged commit 2d9e74b into main Jul 10, 2024
2 checks passed
@wintonzheng wintonzheng deleted the shu/email_relay branch July 10, 2024 00:45
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.

2 participants