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

perf: improve too many user api call #4763

Merged

Conversation

keita-determined
Copy link
Contributor

@keita-determined keita-determined commented Aug 10, 2022

Description

DET-7962

Test Plan

  • Check if user api call is reduced (user api call less than 5 times before polling starts in network tab)
    • user api used to be called more than 5 times before polling starts
  • Check if user data is fetched appropriately instead of no user data

Commentary (optional)

Checklist

  • User-facing API changes need the "User-facing API Change" label.
  • Release notes should be added as a separate file under docs/release-notes/.
    See Release Note for details.
  • Licenses should be included for new code which was copied and/or modified from any external code.
  • If modifying /webui/react/src/shared/ verify make -C webui/react test-shared passes.

@netlify
Copy link

netlify bot commented Aug 10, 2022

Deploy Preview for determined-ui canceled.

Name Link
🔨 Latest commit c8ef508
🔍 Latest deploy log https://app.netlify.com/sites/determined-ui/deploys/62f4949cad45440008fb2c4d

@keita-determined
Copy link
Contributor Author

test-cli is failed. I believe that it should fail cuz the code change is not related to what ci tests


useEffect(() => {
if (!userId) return;
if (!users.length) fetchUsers();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

When lots of user avatars is rendered at once fetchUsers is called many times, so fetchUsers is moved in App.tsx and call once.
This implementation should be fine because lots of components is polling fetchUsers every few seconds

@keita-determined keita-determined assigned gt2345 and unassigned hamidzr Aug 11, 2022
@keita-determined keita-determined removed the request for review from hamidzr August 11, 2022 22:05
@keita-determined keita-determined changed the title fix: improve too many user api call perf: improve too many user api call Aug 12, 2022
Copy link
Contributor

@gt2345 gt2345 left a comment

Choose a reason for hiding this comment

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

LGTM

@keita-determined keita-determined merged commit 7cd0e51 into determined-ai:master Aug 15, 2022
@keita-determined keita-determined deleted the fix/improve-user-fetch branch August 15, 2022 16:48
@dannysauer dannysauer modified the milestones: 0.0.102, 0.19.2 Feb 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants