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 additional db calls when building connections for a workspace #11529

Merged
merged 3 commits into from
Mar 30, 2022

Conversation

malikdiarra
Copy link
Contributor

@malikdiarra malikdiarra commented Mar 29, 2022

What

Improve performance of retrieving the connections of a workspace

How

The current logic makes some unnecessary calls, previous changes have already introduced a way to retrieve all the connections for a workspace. The connections handler continue to do individual calls to retrieve the details of each of those connections which is not needed.

This also fix some tests in ConnectionHandlerTest that broke as part of that change. The tests fixtures have some flaws (standardSync and standardSyncDeleted have the id). I think testing the conversion from StandardSync to ConnectionRead should be part of a test suite for ApiPojoConverter and not that ConnectionHandler.

Recommended reading order

  • ConnectionHandlerTest (change on expectations of the test)
  • ConnectionHandler

@github-actions github-actions bot added area/platform issues related to the platform area/server labels Mar 29, 2022
Copy link
Contributor

@lmossman lmossman left a comment

Choose a reason for hiding this comment

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

LGTM

@malikdiarra malikdiarra temporarily deployed to more-secrets March 30, 2022 21:01 Inactive
@malikdiarra malikdiarra temporarily deployed to more-secrets March 30, 2022 21:01 Inactive
@malikdiarra malikdiarra merged commit 1d2568f into master Mar 30, 2022
@malikdiarra malikdiarra deleted the malik/workspace-get-connections branch March 30, 2022 22:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/platform issues related to the platform area/server
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants