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(platform): Add delete agent functionality #8273

Merged

Conversation

majdyz
Copy link
Contributor

@majdyz majdyz commented Oct 7, 2024

Background

We are not able to remove an agent.

Changes 🏗️

Added UI & API for deleting an Agent.

image image

Testing 🔍

Note

Only for the new autogpt platform, currently in autogpt_platform/

  • Create from scratch and execute an agent with at least 3 blocks
  • Import an agent from file upload, and confirm it executes correctly
  • Upload agent to marketplace
  • Import an agent from marketplace and confirm it executes correctly
  • Edit an agent from monitor, and confirm it executes correctly

@majdyz majdyz requested a review from a team as a code owner October 7, 2024 09:01
@github-actions github-actions bot added platform/frontend AutoGPT Platform - Front end platform/backend AutoGPT Platform - Back end size/l labels Oct 7, 2024
Copy link

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪
🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Error Handling
The new delete_graph function doesn't handle the case where no entries are deleted. Consider adding appropriate error handling or logging for this scenario.

User Experience
The delete functionality lacks a loading state or error handling in the UI. Consider adding these to improve user experience.

Type Consistency
The delete_graph method returns a DeleteGraphResponse with a version_counts field, but it's unclear if this should be plural. Consider renaming to version_count for clarity.

Copy link

netlify bot commented Oct 7, 2024

Deploy Preview for auto-gpt-docs canceled.

Name Link
🔨 Latest commit fd2dbe3
🔍 Latest deploy log https://app.netlify.com/sites/auto-gpt-docs/deploys/67055610002d8100088290b9

@majdyz majdyz enabled auto-merge (squash) October 7, 2024 09:03
Copy link
Contributor

@Swiftyos Swiftyos left a comment

Choose a reason for hiding this comment

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

Just wanted to point out that using TypedDict is causing some issues since our Docker image is based on Python 3.11. Pydantic doesn’t fully support typing.TypedDict in versions below Python 3.12, which is why we're seeing those errors.

To fix this, we need to replace from typing import TypedDict with from typing_extensions import TypedDict. This should make it compatible with our current setup.

@aarushik93
Copy link
Contributor

@Swiftyos why don't we just update the docker image?

@aarushik93
Copy link
Contributor

this clashes with #8272. I can close my PR or you can remove the migrations from yours and use the ones i created

@Swiftyos
Copy link
Contributor

Swiftyos commented Oct 7, 2024

@Swiftyos why don't we just update the docker image?

Both are valid options - I'll let @majdyz decided if he wants to include upgrading the docker image python version in this.

@aarushik93
Copy link
Contributor

@majdyz if you upgrade docker files lets do it in a diff PR please

@majdyz
Copy link
Contributor Author

majdyz commented Oct 7, 2024

@aarushik93 I can remove the change on my end, would you mind to add some of the changes I added on my PR if it's not there already on your PR?

@aarushik93
Copy link
Contributor

@majdyz yeah sure thing - what changes would you like added?

@majdyz
Copy link
Contributor Author

majdyz commented Oct 7, 2024

@aarushik93 nvm ur pr has been auto merged :)

@majdyz majdyz requested a review from Swiftyos October 7, 2024 12:05
@aarushik93
Copy link
Contributor

LGTM - do you wanna get the Docker change in first before merging this though?

@majdyz
Copy link
Contributor Author

majdyz commented Oct 8, 2024

@aarushik93 the change no longer requires higher python version fortunately

@majdyz majdyz merged commit 2a74381 into master Oct 8, 2024
16 checks passed
@majdyz majdyz deleted the zamilmajdy/open-1841-implement-delete-functionality-for-agents branch October 8, 2024 16:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
platform/backend AutoGPT Platform - Back end platform/frontend AutoGPT Platform - Front end Review effort [1-5]: 4 size/l
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants