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

Do not load disabled commands (faster exec & benchmark runs) #5078

Merged
merged 7 commits into from
Jul 30, 2023

Conversation

lc0rp
Copy link
Contributor

@lc0rp lc0rp commented Jul 30, 2023

Background

Disabled commands need to be unregistered after initializing the registry.

get_command_registry() wasn't doing this, leading to more prolonged execution and benchmark runs when the disabled command was loaded, failed, and then the correct one was loaded.

For example (with no Google API key), we typically see:

  • google => fail => web_search => pass

Changes

Swapped get_command_registry for CommandRegistry.with_command_modules

Documentation

N/A

Test Plan

Local testing

PR Quality Checklist

  • My pull request is atomic and focuses on a single change.
  • I have thoroughly tested my changes with multiple different prompts.
  • I have considered potential risks and mitigations for my changes.
  • I have documented my changes clearly and comprehensively.
  • I have not snuck in any "extra" small tweaks changes.
  • I have run the following commands against my code to ensure it passes our linters:
    black .
    isort .
    mypy
    autoflake --remove-all-unused-imports --recursive --ignore-init-module-imports --ignore-pass-after-docstring autogpt tests --in-place

@lc0rp lc0rp requested a review from collijk as a code owner July 30, 2023 05:32
@netlify
Copy link

netlify bot commented Jul 30, 2023

Deploy Preview for auto-gpt-docs failed.

Name Link
🔨 Latest commit f9e5b4b
🔍 Latest deploy log https://app.netlify.com/sites/auto-gpt-docs/deploys/64c5f5ea39d11e0008706902

@lc0rp lc0rp changed the title Do not load disabled commands (reduce exec times & benchmark completion time) Do not load disabled commands (faster exec & benchmark runs) Jul 30, 2023
@Pwuts Pwuts merged commit c1567c2 into Significant-Gravitas:master Jul 30, 2023
@lc0rp lc0rp added this to the v0.4.7 Release milestone Aug 1, 2023
@lc0rp lc0rp deleted the donot-load-disabled-commands branch August 1, 2023 06:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants