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

Replace deprecated event hooks with lifecycle handler #8092

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

vishesh10
Copy link

User description

Background

This PR replaces the deprecated @app.on_event hooks with lifecycle handler.
Addresses #7947

Changes 🏗️

  • Updated the ws_api.py file to have lifecycle handlers instead of on_event hooks.

PR Quality Scorecard ✨

  • Have you used the PR description template?   +2 pts
  • Is your pull request atomic, focusing on a single change?   +5 pts
  • Have you linked the GitHub issue(s) that this PR addresses?   +5 pts
  • Have you documented your changes clearly and comprehensively?   +5 pts
  • Have you changed or added a feature?   -4 pts
    • Have you added/updated corresponding documentation?   +4 pts
    • Have you added/updated corresponding integration tests?   +5 pts
  • Have you changed the behavior of AutoGPT?   -5 pts
    • Have you also run agbenchmark to verify that these changes do not regress performance?   +10 pts

@vishesh10 vishesh10 requested a review from a team as a code owner September 18, 2024 15:45
@vishesh10 vishesh10 requested review from Torantulino and aarushik93 and removed request for a team September 18, 2024 15:45
Copy link

PR-Agent was enabled for this repository. To continue using it, please link your git user with your CodiumAI identity here.

PR Reviewer Guide 🔍

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Key issues to review

Potential Race Condition
The lifespan function creates an asyncio task for event_broadcaster, but doesn't ensure it's properly cancelled on shutdown. This could lead to a race condition or resource leaks.

Copy link

netlify bot commented Sep 18, 2024

Deploy Preview for auto-gpt-docs canceled.

Name Link
🔨 Latest commit 498ee3d
🔍 Latest deploy log https://app.netlify.com/sites/auto-gpt-docs/deploys/66ec565a84aa1e0008e11411

@vishesh10 vishesh10 changed the title Replace decreated event hooks with lifecycle handler Replace deprecated event hooks with lifecycle handler Sep 18, 2024
@aarushik93
Copy link
Contributor

Thanks for your contribution - can you please run the linter?

@Swiftyos
Copy link
Contributor

To run the linter you do:

poetry format in autogpt_sever folder

@vishesh10
Copy link
Author

Thanks for considering this PR @aarushik93 @Swiftyos. The same is done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: 🆕 Needs initial review
Development

Successfully merging this pull request may close these issues.

3 participants