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

Cosmosdb serverless #845

Merged
merged 15 commits into from
Jan 4, 2022

Conversation

amine-mejaouel
Copy link
Contributor

@amine-mejaouel amine-mejaouel commented Dec 12, 2021

This PR closes #333

Thanks to discussion in #333 and for @dnperfors and @JordanMarr work on the subject, I added the last brick in order to make the serverless work on cosmos db.

The changes in this PR are as follows:

  • Update databaseAccounts/* api versions to 2021-04-15
  • Add 'serverless' operation to Cosmos builder
    • Using 'serverless' disables previous 'throughput' usages.
    • Likewise, using 'throughput' disables previous 'serverless' usages.cosmosDb
// Per example, the 'throughput' will be ignored, only the 'serverless' will be applied.
cosmosDb {
   name "cosmos-foo"   
   throughput 400<CosmosDb.RU> 
   serverless
}

I have read the contributing guidelines and have completed the following:

  • Tested my code end-to-end against a live Azure subscription.
  • Updated the documentation in the docs folder for the affected changes.
  • Written unit tests against the modified code that I have made.
  • Updated the release notes with a new entry for this PR.
  • Checked the coding standards outlined in the contributions guide and ensured my code adheres to them.

If I haven't completed any of the tasks above, I include the reasons why here:

Below is a minimal example configuration that includes the new features, which can be used to deploy to Azure:

cosmosDb {
    name "cosmos-foo"
    serverless
}

Copy link
Member

@isaacabraham isaacabraham left a comment

Choose a reason for hiding this comment

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

Great stuff. I think there's a couple of API surface suggestions that I would like to see that would make the API even more compelling, but the majority of this looks good to go.

src/Farmer/Builders/Builders.Cosmos.fs Outdated Show resolved Hide resolved
src/Farmer/Builders/Builders.Cosmos.fs Outdated Show resolved Hide resolved
src/Farmer/Arm/DocumentDb.fs Outdated Show resolved Hide resolved
src/Farmer/Builders/Builders.Cosmos.fs Outdated Show resolved Hide resolved
@ninjarobot ninjarobot added this to the 1.6.24 milestone Dec 19, 2021
@ninjarobot
Copy link
Collaborator

@isaacabraham is this ready to merge?

@isaacabraham
Copy link
Member

Apologies, I need to review the updates.

Copy link
Member

@isaacabraham isaacabraham left a comment

Choose a reason for hiding this comment

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

This is good to go. I made a few tiny tweaks:

  • Removing a when to a proper pattern match (best practice - pattern matches give better guidance for missing cases).
  • Replaced Throughput with FeatureFlag on the Account (correctness - Account doesn't need to know about Throughput, just if Serverless is set or not).
  • Renamed ProvisionedThroughput to simply Provisioned.
  • Added an example in the docs.

@isaacabraham
Copy link
Member

@ninjarobot ok, this is good to go from my side. @amine-mejaouel if you would like to confirm that my final tweaks are ok with you, this can go in. Thanks again!

@amine-mejaouel
Copy link
Contributor Author

It's all good to me,
Thanks @isaacabraham !

@ninjarobot ninjarobot merged commit 3479cb8 into CompositionalIT:master Jan 4, 2022
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.

CosmosDB - add support for new "serverless" option
4 participants