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

refactor(backend): Make block fields consistently use SchemaField #8360

Merged
merged 9 commits into from
Oct 18, 2024

Conversation

majdyz
Copy link
Contributor

@majdyz majdyz commented Oct 17, 2024

Background

Currently, there are a lot of ways to define a field:

  • Just write the type
  • Use pydantic Field
  • Use backend.data.model.SchemaField

The best way to define a field is to use the custom SchemaField which will allow us defining description, placeholder, and default values.

Changes 🏗️

Make all fields use SchemaField. And add check to prevent field defined without a SchemaField.

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 17, 2024 03:39
@majdyz majdyz requested review from Swiftyos and kcze and removed request for a team October 17, 2024 03:39
Copy link

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

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

Potential Performance Issue
The new code checks all fields in input_schema and output_schema, which could be inefficient for large schemas. Consider optimizing this check or performing it only when necessary.

Possible Inconsistency
The 'ratio', 'resolution', and 'frame_rate' fields are changed to use SchemaField, but their descriptions and default values remain the same. Ensure this change is intentional and consistent with other parts of the code.

Naming Conflict
The import 'from backend.data.model import SchemaField as Field' could lead to confusion as it shadows the original Field import. Consider using a different alias or keeping the original name.

@majdyz majdyz requested a review from ntindle October 17, 2024 04:11
@majdyz majdyz requested a review from Swiftyos October 17, 2024 13:43
@majdyz majdyz changed the title fix(backend): Make block fields consistently use SchemaField refactor(backend): Make block fields consistently use SchemaField Oct 18, 2024
@majdyz majdyz merged commit 26b1bca into dev Oct 18, 2024
7 checks passed
@majdyz majdyz deleted the zamilmajdy/make-schema-field-consistent branch October 18, 2024 03:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants