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

Allow creation of multiple Cosmos containers #1984

Merged
merged 6 commits into from
Apr 4, 2020
Merged

Conversation

mdrichardson
Copy link
Contributor

@mdrichardson mdrichardson commented Apr 2, 2020

Fixes #1975

Description

doOnce was preventing cosmosDbPartitionedStorage users from using multiple containers and/or databases. This is because doOnce acted on cosmosDbPartitionedStorage as a whole, rather than individual db/container combinations.

Specific Changes

  • Store a dict of doOnce-awaited functions, keyed by db+container

Testing

Added two tests, which fail without this change and pass with it.

Considerations

Is this the right approach, or would it be better to store the container as a property in cosmosDbPartitionedStorage? My hunch is that there might be concurrency issues with the class property approach.

@coveralls
Copy link

coveralls commented Apr 2, 2020

Pull Request Test Coverage Report for Build 117639

  • 6 of 6 (100.0%) changed or added relevant lines in 2 files are covered.
  • 9 unchanged lines in 2 files lost coverage.
  • Overall coverage decreased (-0.1%) to 79.293%

Files with Coverage Reduction New Missed Lines %
libraries/botframework-streaming/src/webSocket/webSocketTransport.ts 2 80.65%
libraries/botframework-streaming/src/webSocket/webSocketServer.ts 7 66.67%
Totals Coverage Status
Change from base Build 117482: -0.1%
Covered Lines: 9083
Relevant Lines: 11030

💛 - Coveralls

@mdrichardson mdrichardson changed the title Cosmos multiple Allow creation of multiple Cosmos containers Apr 3, 2020
@Stevenic Stevenic merged commit 3d945c5 into master Apr 4, 2020
@Stevenic Stevenic deleted the cosmosMultiple branch April 4, 2020 04:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cannot use different Cosmos DB containers
4 participants