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.

Sign up for GitHub

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

Jump to bottom

fix(ups): add support for multiple clients #405

Merged
merged 24 commits into from
May 13, 2021
Merged

fix(ups): add support for multiple clients #405

merged 24 commits into from
May 13, 2021

Conversation

jaeopt
Copy link
Collaborator

@jaeopt jaeopt commented May 10, 2021

Fix user-profile-service to support multiple clients (sdkKeys).

  • full thread-safety for additional concurrency requirements
  • no resource conflicts for multiple sdkKeys support

@jaeopt
add multi-client tests for datafile handler
e07ee4a
@jaeopt
fix datafile handler sync with atomic dictionary
b4153a0 
fix datafile handler sync with atomic dictionary

cleanup

clean up
@jaeopt
change name for AtomicWrapper
bfabdae
@jaeopt
remove extra line from custom xcode header
e7091de
@jaeopt
add more concurrent tests for datafile handler
6282d4d
@jaeopt
clean up
e21b11d
@jaeopt
change tests to a shared datafile handler
65d11fa
@jaeopt
fix atomic return
826d40b
@jaeopt
clean up
77fb808
@jaeopt
clean up
d6d9fac
@jaeopt
fix per review
e852348
@jaeopt
clean up
e375243
@jaeopt
clean up
5425df3
@jaeopt
clean up
5888f66
@jaeopt
add tests for concurrent event dispatcher
f741634
@jaeopt
clean up tests
181d86f
@jaeopt
add more tests for multi-client event dispatcher
d1dc2fe
@jaeopt
clean up
3a6aad9
@jaeopt
fix conflicts
0ae0fbd
@jaeopt
add tests for multi-client ups
34b0f9f
@jaeopt
add rmwlock for ups
5a73bc3
@jaeopt
add global ups lock
7be7064
@jaeopt
move a shred ups lock to decision-service
0c00ecc
@jaeopt jaeopt requested a review from a team as a code owner May 10, 2021 18:57
@jaeopt jaeopt removed their assignment May 10, 2021
@coveralls
Copy link

Coverage Status

Coverage remained the same at 90.28% when pulling 0c00ecc on jae/multi-ups into b56f9ba on master.

@coveralls
Copy link

coveralls commented May 10, 2021 •
edited
Loading

Coverage Status

Coverage remained the same at 90.28% when pulling 32e3a55 on jae/multi-ups into b56f9ba on master.

yasirfolio3
yasirfolio3 previously approved these changes May 11, 2021
View reviewed changes
Copy link
Contributor

@yasirfolio3 yasirfolio3 left a comment

Choose a reason for hiding this comment

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

LGTM, minor suggestions requested.

Tests/OptimizelyTests-MultiClients/UserProfileServiceTests_MultiClients.swift Show resolved Hide resolved
Tests/OptimizelyTests-MultiClients/UserProfileServiceTests_MultiClients.swift Outdated Show resolved Hide resolved
msohailhussain
msohailhussain approved these changes May 13, 2021
View reviewed changes
Copy link
Contributor

@msohailhussain msohailhussain left a comment

Choose a reason for hiding this comment

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

lgtm

@jaeopt jaeopt merged commit a3a8c14 into master May 13, 2021
@jaeopt jaeopt deleted the jae/multi-ups branch May 13, 2021 18:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@msohailhussain msohailhussain msohailhussain approved these changes

@yasirfolio3 yasirfolio3 yasirfolio3 left review comments

Assignees
No one assigned
Labels
🤖 20 hrs
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@jaeopt @coveralls @msohailhussain @yasirfolio3 @optimizely-codereview-bot