Skip to content

Conversation

@nbbeeken
Copy link
Contributor

Added the authorizedDatabases option to ListDatabasesOperation as an optional param. No changes needed to the helpers.
Check out the flag docs here

@nbbeeken nbbeeken requested review from kmahar and patrickfreed April 15, 2020 15:25
@kmahar
Copy link
Contributor

kmahar commented Apr 16, 2020

just occurred to me, you'll need to make these same API changes to support ListDatabasesOptions in the sync API as well!
and you need to run make exports so that ListDatabasesOptions gets re-exported from the sync module

Copy link
Contributor

@kmahar kmahar left a comment

Choose a reason for hiding this comment

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

looking good 👍 just a few more nits

* - `LogicError` if the provided session is inactive.
* - `LogicError` if this client has already been closed.
* - `EncodingError` if an error is encountered while encoding the options to BSON.
* - `CommandError` when options.authorizedDatabases is false:
Copy link
Contributor

Choose a reason for hiding this comment

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

sorry to be pedantic...

  1. can we phrase this more similarly to the previous bullet points? by that I mean the "CommandError if ...". format. so something like "if options.authorizedDatabases is false and the user does not have listDatabases permissions."

  2. listDatabase -> listDatabases

Copy link
Contributor

@kmahar kmahar 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 d026dd9 into master Apr 20, 2020
@nbbeeken nbbeeken deleted the SWIFT-773/authorizedDatabases branch April 20, 2020 21:04
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