-
Notifications
You must be signed in to change notification settings - Fork 281
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
Cosmos partitioning #1282
Cosmos partitioning #1282
Conversation
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have some suggestions, but overall looks good. 👍
@christopheranderson Great feedback! I should be able to fix those tomorrow. |
Pull Request Test Coverage Report for Build 83263
💛 - Coveralls |
Please merge when ready. |
@cleemullins Will do. Implementing @christopheranderson's recommendations now and will merge once build tests passes. However, coveralls test coverage won't allow merge. I believe the issue is that most of the tests don't actually run since there's no Storage Emulator. Is that something we can bypass? ...I don't believe any of the JS storage tests actually run the read/write/deletes in the pipeline tests. |
@mdrichardson @christopheranderson Let's discuss handling non partitioned etc tomorrow when you get online. I have some specific thoughts here. |
…nto cosmosPartitioning
…botbuilder-js into cosmosPartitioning
Fixes #1234
Description
Parity with Dotnet, adding a separate class for supporting Partitioned Containers on Cosmos.
Specific Changes
@azure/cosmos
SDKAlso implements these additional changes from the Dotnet PR
Testing
Added new, separate tests.
On a side note: I plan to eventually create a StorageBaseTests class for the JS repo, so we remove a lot of the duplicate code. Wanted to get this out for 4.6, though.