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

shu/removeSettingsManager.get_settings #1305

Merged
merged 1 commit into from
Dec 2, 2024

Conversation

wintonzheng
Copy link
Contributor

@wintonzheng wintonzheng commented Dec 2, 2024


Important

Replaces SettingsManager.get_settings() with settings across multiple files for configuration access and adds Gemini API settings.

  • Behavior:
    • Replaces SettingsManager.get_settings() with settings in action_input.py, url.py, and retry_llm_request.py for accessing configuration settings.
    • Updates run_task.py, run_workflow.py, and analytics.py to use settings for configuration access.
  • Configuration:
    • Modifies config.py to include new settings ENABLE_GEMINI and GEMINI_API_KEY.
  • Misc:
    • Updates __main__.py, api_app.py, and aws.py to use settings for configuration.
    • Changes files.py, llm/api_handler_factory.py, and llm/config_registry.py to use settings.
    • Updates local.py, security.py, and forge_log.py to use settings.
    • Modifies agent_protocol.py, org_auth_service.py, and block.py to use settings.
    • Updates service.py, handler.py, and models.py to use settings.
    • Changes browser_factory.py, scraper.py, and dom.py to use settings.
    • Updates page.py and temporal_activities.py to use settings.

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

- commit cloud/webeye/setup/action_input.py
- commit
- commit
- commit
- commit
- commit
- commit
- commit
- commit 2 more
- commit more
- commit more

<!-- ELLIPSIS_HIDDEN -->

----

> [!IMPORTANT]
> Replaces `SettingsManager.get_settings()` with `settings` for configuration access and adds Gemini API settings.
>
>   - **Behavior**:
>     - Replaces `SettingsManager.get_settings()` with `settings` in `action_input.py`, `url.py`, and `retry_llm_request.py` for accessing configuration settings.
>     - Updates `run_task.py`, `run_workflow.py`, and `analytics.py` to use `settings` for configuration access.
>   - **Configuration**:
>     - Modifies `config.py` to include new settings `ENABLE_GEMINI` and `GEMINI_API_KEY`.
>   - **Misc**:
>     - Updates `__main__.py`, `api_app.py`, and `aws.py` to use `settings` for configuration.
>     - Changes `files.py`, `llm/api_handler_factory.py`, and `llm/config_registry.py` to use `settings`.
>     - Updates `local.py`, `security.py`, and `forge_log.py` to use `settings`.
>     - Modifies `agent_protocol.py`, `org_auth_service.py`, and `block.py` to use `settings`.
>     - Updates `service.py`, `handler.py`, and `models.py` to use `settings`.
>     - Changes `browser_factory.py`, `scraper.py`, and `dom.py` to use `settings`.
>     - Updates `page.py` and `temporal_activities.py` to use `settings`.
>
> <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 ad83813b3e5000712f5236dcb3619028d3cd2370. 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! Reviewed everything up to 5139d34 in 37 seconds

More details
  • Looked at 1414 lines of code in 23 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. skyvern/webeye/actions/handler.py:15
  • Draft comment:
    Ensure that settings is properly initialized and imported in all contexts where it is used. This change replaces SettingsManager.get_settings() with settings, which assumes settings is always available and correctly configured.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The PR replaces SettingsManager.get_settings() with settings for configuration access. This change is consistent across the files, ensuring a uniform approach to accessing settings. However, there is a potential issue with the use of settings directly without ensuring that it is properly initialized or imported in all contexts. This could lead to runtime errors if settings is not available or correctly configured in some parts of the code.

Workflow ID: wflow_bQSulrb6rHOU8hBK


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 5139d34 in 1 minute and 5 seconds

More details
  • Looked at 1414 lines of code in 23 files
  • Skipped 0 files when reviewing.
  • Skipped posting 6 drafted comments based on config settings.
1. skyvern/webeye/actions/handler.py:374
  • Draft comment:
    Ensure that settings.BROWSER_ACTION_TIMEOUT_MS is defined and correctly configured in skyvern.config. This change replaces SettingsManager.get_settings().BROWSER_ACTION_TIMEOUT_MS with settings.BROWSER_ACTION_TIMEOUT_MS, which should be verified for correctness.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The PR replaces SettingsManager.get_settings() with settings for configuration access. This change is consistent across multiple files, including handler.py, models.py, browser_factory.py, scraper.py, dom.py, and page.py. The change is straightforward and aligns with the PR description.
2. skyvern/webeye/actions/models.py:48
  • Draft comment:
    Ensure that settings.DEBUG_MODE is defined and correctly configured in skyvern.config. This change replaces SettingsManager.get_settings().DEBUG_MODE with settings.DEBUG_MODE, which should be verified for correctness.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The PR replaces SettingsManager.get_settings() with settings for configuration access. This change is consistent across multiple files, including handler.py, models.py, browser_factory.py, scraper.py, dom.py, and page.py. The change is straightforward and aligns with the PR description.
3. skyvern/webeye/browser_factory.py:145
  • Draft comment:
    Ensure that settings.VIDEO_PATH is defined and correctly configured in skyvern.config. This change replaces SettingsManager.get_settings().VIDEO_PATH with settings.VIDEO_PATH, which should be verified for correctness.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The PR replaces SettingsManager.get_settings() with settings for configuration access. This change is consistent across multiple files, including handler.py, models.py, browser_factory.py, scraper.py, dom.py, and page.py. The change is straightforward and aligns with the PR description.
4. skyvern/webeye/scraper/scraper.py:304
  • Draft comment:
    Ensure that settings.MAX_SCRAPING_RETRIES is defined and correctly configured in skyvern.config. This change replaces SettingsManager.get_settings().MAX_SCRAPING_RETRIES with settings.MAX_SCRAPING_RETRIES, which should be verified for correctness.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The PR replaces SettingsManager.get_settings() with settings for configuration access. This change is consistent across multiple files, including handler.py, models.py, browser_factory.py, scraper.py, dom.py, and page.py. The change is straightforward and aligns with the PR description.
5. skyvern/webeye/utils/dom.py:316
  • Draft comment:
    Ensure that settings.BROWSER_ACTION_TIMEOUT_MS is defined and correctly configured in skyvern.config. This change replaces SettingsManager.get_settings().BROWSER_ACTION_TIMEOUT_MS with settings.BROWSER_ACTION_TIMEOUT_MS, which should be verified for correctness.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The PR replaces SettingsManager.get_settings() with settings for configuration access. This change is consistent across multiple files, including handler.py, models.py, browser_factory.py, scraper.py, dom.py, and page.py. The change is straightforward and aligns with the PR description.
6. skyvern/webeye/utils/page.py:40
  • Draft comment:
    Ensure that settings.BROWSER_ACTION_TIMEOUT_MS is defined and correctly configured in skyvern.config. This change replaces SettingsManager.get_settings().BROWSER_ACTION_TIMEOUT_MS with settings.BROWSER_ACTION_TIMEOUT_MS, which should be verified for correctness.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The PR replaces SettingsManager.get_settings() with settings for configuration access. This change is consistent across multiple files, including handler.py, models.py, browser_factory.py, scraper.py, dom.py, and page.py. The change is straightforward and aligns with the PR description.

Workflow ID: wflow_JFszqggPGVTZXMLH


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

@wintonzheng wintonzheng merged commit 7f6b2c0 into main Dec 2, 2024
2 checks passed
@wintonzheng wintonzheng deleted the shu/removeSettingsManager.get_settings branch December 2, 2024 23:01
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