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

feat: add screenshotone block #9308

Merged
merged 33 commits into from
Feb 3, 2025
Merged

feat: add screenshotone block #9308

merged 33 commits into from
Feb 3, 2025

Conversation

ntindle
Copy link
Member

@ntindle ntindle commented Jan 21, 2025

I want to be able to screenshot things

Changes 🏗️

Adds ScreenshotOne blocks with the ability to screenshot stuff

Checklist 📋

For code changes:

  • I have clearly listed my changes in the PR description
  • I have made a test plan
  • I have tested my changes according to the test plan:
    • Built agent with it

@ntindle ntindle requested a review from a team as a code owner January 21, 2025 16:01
@ntindle ntindle requested review from Pwuts and majdyz and removed request for a team January 21, 2025 16:01
@github-actions github-actions bot added platform/frontend AutoGPT Platform - Front end platform/backend AutoGPT Platform - Back end platform/blocks size/l labels Jan 21, 2025
Copy link

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🧪 PR contains tests
🔒 Security concerns

API Key Exposure:
The API key is sent as a URL parameter in the request to ScreenshotOne API. While using HTTPS, the API key could still be exposed in server logs or browser history. Consider using header-based authentication if supported by the API.

⚡ Recommended focus areas for review

Error Handling

The error handling in the run() method is very basic and could be improved to provide more specific error messages for different failure scenarios (network issues, invalid API key, etc)

try:
    screenshot_data = self.take_screenshot(
        credentials=credentials,
        url=input_data.url,
        viewport_width=input_data.viewport_width,
        viewport_height=input_data.viewport_height,
        full_page=input_data.full_page,
        format=input_data.format,
        block_ads=input_data.block_ads,
        block_cookie_banners=input_data.block_cookie_banners,
        block_chats=input_data.block_chats,
        cache=input_data.cache,
    )
    yield "image_data", screenshot_data["image_data"]
    yield "content_type", screenshot_data["content_type"]
except Exception as e:
    yield "error", str(e)
Input Validation

The format parameter accepts any string value but should be restricted to only valid formats (png, jpeg, webp). Consider adding input validation or using an enum.

format: str = SchemaField(
    description="Output format (png, jpeg, webp)", default="png"
)

Copy link

netlify bot commented Jan 21, 2025

Deploy Preview for auto-gpt-docs-dev canceled.

Name Link
🔨 Latest commit 6838c62
🔍 Latest deploy log https://app.netlify.com/sites/auto-gpt-docs-dev/deploys/67a0d106ef43a0000817d9bb

@ntindle ntindle changed the title test(frontend): Re-enable the tests in monitor.spec.ts and then ensur… feat: add screenshotone block Jan 21, 2025
Copy link

netlify bot commented Jan 21, 2025

Deploy Preview for auto-gpt-docs canceled.

Name Link
🔨 Latest commit 6838c62
🔍 Latest deploy log https://app.netlify.com/sites/auto-gpt-docs/deploys/67a0d106bfd8f60008508c40

@ntindle ntindle enabled auto-merge January 23, 2025 03:18
majdyz
majdyz previously approved these changes Jan 24, 2025
autogpt_platform/backend/backend/blocks/screenshotone.py Outdated Show resolved Hide resolved
autogpt_platform/backend/backend/blocks/screenshotone.py Outdated Show resolved Hide resolved
autogpt_platform/backend/backend/blocks/screenshotone.py Outdated Show resolved Hide resolved
Copy link
Contributor

This pull request has conflicts with the base branch, please resolve those so we can evaluate the pull request.

@github-actions github-actions bot added the conflicts Automatically applied to PRs with merge conflicts label Jan 28, 2025
@github-actions github-actions bot removed the conflicts Automatically applied to PRs with merge conflicts label Jan 28, 2025
Copy link
Contributor

Conflicts have been resolved! 🎉 A maintainer will review the pull request shortly.

@ntindle ntindle enabled auto-merge January 28, 2025 08:50
@ntindle ntindle disabled auto-merge January 28, 2025 08:50
@aarushik93
Copy link
Contributor

will this display the screenshot on the UI too?

Copy link
Contributor

This pull request has conflicts with the base branch, please resolve those so we can evaluate the pull request.

@github-actions github-actions bot added the conflicts Automatically applied to PRs with merge conflicts label Jan 28, 2025
@github-actions github-actions bot removed the conflicts Automatically applied to PRs with merge conflicts label Jan 29, 2025
Copy link
Contributor

Conflicts have been resolved! 🎉 A maintainer will review the pull request shortly.

Copy link

deepsource-io bot commented Jan 30, 2025

Here's the code health analysis summary for commits 7b50e9b..6838c62. View details on DeepSource ↗.

Analysis Summary

AnalyzerStatusSummaryLink
DeepSource JavaScript LogoJavaScript✅ Success
❗ 48 occurences introduced
🎯 48 occurences resolved
View Check ↗
DeepSource Python LogoPython✅ Success
❗ 2 occurences introduced
🎯 1 occurence resolved
View Check ↗

💡 If you’re a repository administrator, you can configure the quality gates from the settings.

@ntindle
Copy link
Member Author

ntindle commented Feb 3, 2025

will this display the screenshot on the UI too?

image

@ntindle ntindle requested a review from aarushik93 February 3, 2025 14:22
@ntindle ntindle enabled auto-merge February 3, 2025 14:22
@ntindle ntindle added this pull request to the merge queue Feb 3, 2025
Merged via the queue into dev with commit 4302c5d Feb 3, 2025
26 checks passed
@ntindle ntindle deleted the screenshotone branch February 3, 2025 15:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants