Skip to content

Conversation

@rose-m
Copy link
Contributor

@rose-m rose-m commented Jan 15, 2021

Description

What changed?
This updates the type definition of the AutoEncryptionOptions to support GCP and Azure KMS providers. This is in line with mongodb/libmongocrypt#154.

Are there any files to ignore?
None

Copy link
Member

@durran durran left a comment

Choose a reason for hiding this comment

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

LGTM

@rose-m
Copy link
Contributor Author

rose-m commented Jan 18, 2021

I'll quickly update again to reflect the "more strict" typings I've used in libmongocrypt according to the spec.

@rose-m rose-m force-pushed the MONGOSH-527-update-auto-encryption-typings branch from cfbe0e7 to bf849c6 Compare January 18, 2021 16:33
@nbbeeken
Copy link
Contributor

@rose-m If we merge and release the types in mongodb-client-encryption then can we re-export those? Are they identical?

@rose-m
Copy link
Contributor Author

rose-m commented Jan 19, 2021

Hey @nbbeeken - yeah I've essentially literally copied over the KMSProviders type content. Once the types from mongodb-client-encryption are published, we could substitute that with kmsProviders?: KMSProviders, too, and get rid of the whole block.

Copy link
Contributor

@nbbeeken nbbeeken left a comment

Choose a reason for hiding this comment

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

Actually I realize I think we'll want to keep these in the driver since CSFLE is an optional dependency, we wouldn't want to make it required just for the types. Regardless, LGTM 🚀

Copy link
Contributor

@emadum emadum left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@nbbeeken nbbeeken merged commit bcacf5f into mongodb:master Jan 19, 2021
@rose-m rose-m deleted the MONGOSH-527-update-auto-encryption-typings branch January 20, 2021 08:17
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.

4 participants