Skip to content

Comments

Move DB interactions for client to wire-subsystems#4984

Open
akshaymankar wants to merge 6 commits intodevelopfrom
client-store
Open

Move DB interactions for client to wire-subsystems#4984
akshaymankar wants to merge 6 commits intodevelopfrom
client-store

Conversation

@akshaymankar
Copy link
Member

@akshaymankar akshaymankar commented Jan 27, 2026

Checklist

  • Add a new entry in an appropriate subdirectory of changelog.d No changelog.
  • Read and follow the PR guidelines

@zebot zebot added the ok-to-test Approved for running tests in CI, overrides not-ok-to-test if both labels exist label Jan 27, 2026
@akshaymankar akshaymankar reopened this Feb 23, 2026
@akshaymankar akshaymankar changed the base branch from develop to user-client-index-store February 23, 2026 14:21
@akshaymankar akshaymankar force-pushed the client-store branch 3 times, most recently from c7236cd to df2205a Compare February 24, 2026 10:46
Base automatically changed from user-client-index-store to develop February 24, 2026 11:05
@akshaymankar akshaymankar marked this pull request as ready for review February 24, 2026 14:37
@akshaymankar akshaymankar requested a review from a team as a code owner February 24, 2026 14:37
Copy link
Contributor

@battermann battermann left a comment

Choose a reason for hiding this comment

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

Only a minor comment. Otherwise LGTM.

allClients <- lift (wrapClient (API.lookupUsersClientIds (pure usr))) >>= getResult
clientInfos <- lift . wrapClient $ UnliftIO.Async.pooledMapConcurrentlyN 16 (\c -> getMLSClient lusr c suiteTag) (toList allClients)
allClients <- lift (liftSem (ClientStore.lookupClientIds usr))
clientInfos <- lift $ traverse (\c -> getMLSClient lusr c suiteTag) (toList allClients)
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we keep the parallel lookup here? Even with max 8 clients, this should be a significant improvement.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ok-to-test Approved for running tests in CI, overrides not-ok-to-test if both labels exist

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants