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

optimize svg cache and add retry #752

Merged
merged 1 commit into from
Aug 29, 2024
Merged

Conversation

LawyZheng
Copy link
Collaborator

@LawyZheng LawyZheng commented Aug 29, 2024

🚀 This description was created by Ellipsis for commit e4af622

Summary:

This PR optimizes SVG conversion by caching results and retrying failed conversions up to 3 times, introducing a new exception for handling conversion failures.

Key points:

  • Added SVGConversionFailed exception in skyvern/exceptions.py for handling failed SVG conversions.
  • Introduced USELESS_SVG_ATTRIBUTE and SVG_RETRY_ATTEMPT constants in skyvern/forge/agent_functions.py.
  • Modified _remove_skyvern_attributes to remove unnecessary SVG attributes.
  • Updated _convert_svg_to_string to retry SVG conversion up to 3 times before raising SVGConversionFailed.
  • Improved logging for SVG conversion attempts and failures.

Generated with ❤️ by ellipsis.dev

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 e4af622 in 16 seconds

More details
  • Looked at 100 lines of code in 2 files
  • Skipped 0 files when reviewing.
  • Skipped posting 3 drafted comments based on config settings.
1. skyvern/forge/agent_functions.py:27
  • Draft comment:
    Consider adding a type hint for the element parameter.
def _remove_rect(element: Dict) -> None:
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The code is missing type hints for the element parameter in the _remove_rect function. Adding type hints improves code readability and helps with static analysis.
2. skyvern/forge/agent_functions.py:42
  • Draft comment:
    Consider adding a type hint for the element parameter.
def _remove_skyvern_attributes(element: Dict) -> Dict:
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The code is missing type hints for the element parameter in the _remove_skyvern_attributes function. Adding type hints improves code readability and helps with static analysis.
3. skyvern/forge/agent_functions.py:87
  • Draft comment:
    Consider adding a type hint for the element parameter.
async def _convert_svg_to_string(task: Task, step: Step, organization: Organization | None, element: Dict) -> None:
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The code is missing type hints for the element parameter in the _convert_svg_to_string function. Adding type hints improves code readability and helps with static analysis.

Workflow ID: wflow_K4DTpMyORurSHL83


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

@LawyZheng LawyZheng merged commit f115202 into main Aug 29, 2024
2 checks passed
@LawyZheng LawyZheng deleted the lawy/svg-cache-and-retry branch August 29, 2024 03:11
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