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

Remove config singleton #4737

Merged

Conversation

waynehamadi
Copy link
Contributor

@waynehamadi waynehamadi commented Jun 19, 2023

Background

There were a few more calls to config() in the codebase.

Changes

  • removed remaining calls to config()
  • removed config being a singleton. This means it's not allowed to call it initialize it twice in the autogpt folder. (we still initialize it once in pytest.
  • removed flaky because it's slowing down the pipeline: a flaky challenge should be discarded from the pipeline anyway, there is no point keeping it.
  • the service unavailable 503 error is a new error OpenAI has been giving recently, we want to retry that so it's been added to the retry_api decorator.

Documentation

Test Plan

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

@waynehamadi waynehamadi marked this pull request as draft June 19, 2023 01:51
@github-actions
Copy link
Contributor

This PR exceeds the recommended size of 500 lines. Please make sure you are NOT addressing multiple issues with one PR.

@netlify
Copy link

netlify bot commented Jun 19, 2023

Deploy Preview for auto-gpt-docs ready!

Name Link
🔨 Latest commit 029310c
🔍 Latest deploy log https://app.netlify.com/sites/auto-gpt-docs/deploys/64910f6f6d3e8800083f0017
😎 Deploy Preview https://deploy-preview-4737--auto-gpt-docs.netlify.app/
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@github-actions
Copy link
Contributor

This PR exceeds the recommended size of 500 lines. Please make sure you are NOT addressing multiple issues with one PR.

@github-actions
Copy link
Contributor

This PR exceeds the recommended size of 500 lines. Please make sure you are NOT addressing multiple issues with one PR.

@github-actions
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 Jun 19, 2023
@github-actions github-actions bot added size/m and removed size/xl labels Jun 19, 2023
@github-actions github-actions bot removed the conflicts Automatically applied to PRs with merge conflicts label Jun 19, 2023
@github-actions
Copy link
Contributor

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

@waynehamadi waynehamadi marked this pull request as ready for review June 19, 2023 03:46
@waynehamadi waynehamadi changed the title Remove singleton Remove config singleton Jun 19, 2023
@waynehamadi waynehamadi force-pushed the remove-singleton branch 3 times, most recently from f7ec4d7 to 847a181 Compare June 19, 2023 14:34
@codecov
Copy link

codecov bot commented Jun 19, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: -0.17 ⚠️

Comparison is base (ea1b28f) 70.91% compared to head (029310c) 70.75%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4737      +/-   ##
==========================================
- Coverage   70.91%   70.75%   -0.17%     
==========================================
  Files          67       67              
  Lines        3239     3238       -1     
  Branches      513      513              
==========================================
- Hits         2297     2291       -6     
- Misses        777      780       +3     
- Partials      165      167       +2     
Impacted Files Coverage Δ
autogpt/config/config.py 72.32% <100.00%> (-0.18%) ⬇️

... and 4 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@waynehamadi waynehamadi force-pushed the remove-singleton branch 4 times, most recently from d771011 to 5f59396 Compare June 19, 2023 15:40
erik-megarad
erik-megarad previously approved these changes Jun 19, 2023
@github-actions github-actions bot added size/l conflicts Automatically applied to PRs with merge conflicts labels Jun 19, 2023
@github-actions
Copy link
Contributor

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

@Auto-GPT-Bot
Copy link
Contributor

You changed AutoGPT's behaviour. The cassettes have been updated and will be merged to the submodule when this Pull Request gets merged.

@github-actions github-actions bot removed the conflicts Automatically applied to PRs with merge conflicts label Jun 20, 2023
@github-actions
Copy link
Contributor

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

@ntindle ntindle added this to the v0.4.2 Release milestone Jun 20, 2023
@waynehamadi
Copy link
Contributor Author

You will be missed, singleton.

@waynehamadi waynehamadi merged commit 4e3f832 into Significant-Gravitas:master Jun 20, 2023
@collijk collijk mentioned this pull request Jun 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants