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

fix(rnd): Added updated graph meta to include runs #8088

Conversation

Swiftyos
Copy link
Contributor

@Swiftyos Swiftyos commented Sep 18, 2024

Background

Fixes the large number of requests on the monitoring page, by expanding the get graphs call to include minimal runs meta. Thus we get all the data for the monitoring page in a single request.

Also:

  • Tidied up pyproject.toml file to remove none existent build steps
  • Made sentry toggle able

@Swiftyos Swiftyos requested a review from a team as a code owner September 18, 2024 14:30
@Swiftyos Swiftyos requested review from Torantulino and Bentlybro and removed request for a team September 18, 2024 14:30
@github-actions github-actions bot added platform/frontend AutoGPT Platform - Front end platform/backend AutoGPT Platform - Back end size/l labels Sep 18, 2024
Copy link

PR Reviewer Guide 🔍

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

Possible Performance Issue
The useEffect hook is called without dependencies, which may cause unnecessary re-renders. Consider adding the appropriate dependencies to the dependency array.

Potential Memory Leak
The setInterval is set up in a useEffect hook that depends on fetchAgents and agents. This might cause multiple intervals to be created if these dependencies change frequently. Consider using a useRef to store the interval ID and clear it properly.

Copy link

netlify bot commented Sep 18, 2024

Deploy Preview for auto-gpt-docs ready!

Name Link
🔨 Latest commit c388385
🔍 Latest deploy log https://app.netlify.com/sites/auto-gpt-docs/deploys/66f1355849fb2e000887ef9a
😎 Deploy Preview https://deploy-preview-8088--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 configuration.

kcze
kcze previously requested changes Sep 18, 2024
Copy link
Contributor

@kcze kcze left a comment

Choose a reason for hiding this comment

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

This PR renames some things from flow to agent but there are still inconsistencies. I think using different words for the same thing is confusing for us and contributors.

rnd/autogpt_server/autogpt_server/data/graph.py Outdated Show resolved Hide resolved
rnd/autogpt_server/autogpt_server/data/graph.py Outdated Show resolved Hide resolved
rnd/autogpt_builder/src/app/page.tsx Outdated Show resolved Hide resolved
@Swiftyos
Copy link
Contributor Author

This PR renames some things from flow to agent but there are still inconsistencies. I think using different words for the same thing is confusing for us and contributors.

I agree, I was working on bring down the scope of concepts we have. We use Agent, Flow and Graph all interchangeable atm

Given we only talk to endusers with the term agent I was trying to standardise on that.

Would you like me to propagate the name change across the system?

@Swiftyos
Copy link
Contributor Author

This PR renames some things from flow to agent but there are still inconsistencies. I think using different words for the same thing is confusing for us and contributors.

I agree, I was working on bring down the scope of concepts we have. We use Agent, Flow and Graph all interchangeable atm

Given we only talk to endusers with the term agent I was trying to standardise on that.

Would you like me to propagate the name change across the system?

I decided it was best to change the naming schema in a different PR

@Swiftyos Swiftyos requested a review from kcze September 19, 2024 08:05
@Swiftyos Swiftyos dismissed kcze’s stale review September 19, 2024 09:12

Can't find the request re-review button..

@majdyz
Copy link
Contributor

majdyz commented Sep 19, 2024

I'm still seeing this:

Screen.Recording.2024-09-19.at.12.01.42.PM.mov

Also seeing another error:

Screen.Recording.2024-09-19.at.12.04.01.PM.mov

@majdyz
Copy link
Contributor

majdyz commented Sep 19, 2024

oh nvm it's fixed on the latest changes

Pwuts
Pwuts previously approved these changes Sep 20, 2024
Copy link
Member

@Pwuts Pwuts left a comment

Choose a reason for hiding this comment

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

LGTM!

@Swiftyos Swiftyos changed the base branch from master to repo-restructure September 20, 2024 12:55
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 Sep 20, 2024
Base automatically changed from repo-restructure to master September 20, 2024 14:50
@Swiftyos Swiftyos dismissed Pwuts’s stale review September 20, 2024 14:50

The base branch was changed.

… swiftyos/secrt-871-monitor-page-crashing-on-due-to-number-of-requests
…er-of-requests' of github.com:Significant-Gravitas/AutoGPT into swiftyos/secrt-871-monitor-page-crashing-on-due-to-number-of-requests
Copy link
Contributor

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

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

majdyz commented Sep 20, 2024

@Swiftyos This fix seems to be crucial.
I have resolved the conflict on this PR, try to rebase and let me know if it is working as expected.

@Swiftyos
Copy link
Contributor Author

Tested these routes merging

  • 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
  • Edit an agent from monitor, and confirm it executes correctly

@Swiftyos Swiftyos merged commit c07cf8a into master Sep 23, 2024
13 checks passed
@Swiftyos Swiftyos deleted the swiftyos/secrt-871-monitor-page-crashing-on-due-to-number-of-requests branch September 23, 2024 09:31
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]: 3 size/l
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

5 participants