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(blocks): add base for smartlead, apollo, and zerobounce blocks #9387

Open
wants to merge 21 commits into
base: dev
Choose a base branch
from

Conversation

ntindle
Copy link
Member

@ntindle ntindle commented Jan 31, 2025

We want to support some more advanced search specific actions. These are the base API layers and sample blocks for some of the services we need.

Changes 🏗️

  • support pydantic models as an output format
  • add apollo
  • add smartlead
  • add zerobounce

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 agents to test

@ntindle ntindle requested a review from a team as a code owner January 31, 2025 23:51
@ntindle ntindle requested review from Swiftyos and Bentlybro and removed request for a team January 31, 2025 23:51
@github-actions github-actions bot added the conflicts Automatically applied to PRs with merge conflicts label Jan 31, 2025
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 platform/frontend AutoGPT Platform - Front end platform/backend AutoGPT Platform - Back end platform/blocks size/xl labels Jan 31, 2025
Copy link

netlify bot commented Jan 31, 2025

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

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

Copy link

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪
🧪 No relevant tests
🔒 Security concerns

API Key Exposure:
The SmartLead API implementation exposes the API key in the URL (smartlead/_api.py). This is a security risk as API keys in URLs can be logged by servers and appear in browser histories. The API key should be moved to request headers or body.

⚡ Recommended focus areas for review

Security Concern

The API key is exposed in the URL which is a security risk. Consider moving it to request headers or body.

# This api is stupid and requires your api key in the url. DO NOT RAISE ERRORS FOR BAD REQUESTS.
# FILTER OUT THE API KEY FROM THE ERROR MESSAGE.
Error Handling

The search_people and search_organizations methods lack proper error handling for API failures.

def search_people(self, query: SearchPeopleRequest) -> List[Contact]:
    """Search for people in Apollo"""
    response = self.requests.get(
        f"{self.API_URL}/mixed_people/search",
        headers=self._get_headers(),
        params=query.model_dump(exclude={"credentials"}),
    )
    response_json = response.json()
    parsed_response = SearchPeopleResponse(**response_json)
    if parsed_response.pagination.total_entries == 0:
        return []
    return parsed_response.people
Input Validation

The ip_address parameter is not validated for proper IP address format before being sent to the API.

ip_address: str = SchemaField(
    description="IP address to validate",
    default="",
)

Copy link

deepsource-io bot commented Jan 31, 2025

Here's the code health analysis summary for commits f6e395f..bdcf40d. View details on DeepSource ↗.

Analysis Summary

AnalyzerStatusSummaryLink
DeepSource JavaScript LogoJavaScript✅ SuccessView Check ↗
DeepSource Python LogoPython✅ Success
❗ 4 occurences introduced
🎯 2 occurences resolved
View Check ↗

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

Copy link

netlify bot commented Jan 31, 2025

Deploy Preview for auto-gpt-docs canceled.

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

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

github-actions bot commented Feb 1, 2025

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

Copy link
Contributor

@kcze kcze left a comment

Choose a reason for hiding this comment

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

  1. poetry.lock complains
  2. Remove smartlead for now due to leakage?

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

github-actions bot commented Feb 3, 2025

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

Copy link
Contributor

github-actions bot commented Feb 3, 2025

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

@github-actions github-actions bot removed the conflicts Automatically applied to PRs with merge conflicts label Feb 3, 2025
@ntindle ntindle requested a review from kcze February 3, 2025 16:39
@piyyy314 piyyy314 mentioned this pull request Feb 5, 2025
@github-actions github-actions bot added the conflicts Automatically applied to PRs with merge conflicts label Feb 5, 2025
Copy link
Contributor

github-actions bot commented Feb 5, 2025

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

Copy link
Contributor

github-actions bot commented Feb 5, 2025

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

@github-actions github-actions bot added conflicts Automatically applied to PRs with merge conflicts and removed conflicts Automatically applied to PRs with merge conflicts labels Feb 5, 2025
Copy link
Contributor

github-actions bot commented Feb 6, 2025

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

@aarushik93
Copy link
Contributor

I've approved, I do think we should discuss do we want to map 1-1 the variables external services have named or do we want our own names, which might make more sense. For example I don't like the q prefix for query and ok lol

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
conflicts Automatically applied to PRs with merge conflicts platform/backend AutoGPT Platform - Back end platform/blocks platform/frontend AutoGPT Platform - Front end Review effort [1-5]: 4 size/xl
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

3 participants