feat: support CosmosDB SDK-type bindings #109
Conversation
...tions-extensions-bindings-cosmos/azurefunctions/extensions/bindings/cosmos/containerProxy.py
Outdated
Show resolved
Hide resolved
EvanR-Dev
requested review from
vrdmr,
gavin-aguiar and
hallvictoria
as code owners
...tions-extensions-bindings-cosmos/azurefunctions/extensions/bindings/cosmos/containerProxy.py
Outdated
Show resolved
Hide resolved
...tions-extensions-bindings-cosmos/azurefunctions/extensions/bindings/cosmos/containerProxy.py
Outdated
Show resolved
Hide resolved
azurefunctions-extensions-bindings-cosmos/samples/cosmos_samples_containerproxy/function_app.py
Outdated
Show resolved
Hide resolved
|
|
||
|
|
||
| @app.route(route="container") | ||
| @app.cosmos_db_input(arg_name="container", |
There was a problem hiding this comment.
But these aren't docs right? The arg_name is the name of the client being used at the time for the deferred binding. E.g. container is for the name of the ContainerProxy client coming from the SDK. I feel we keep this or even rename all arg_names to client
...nctions-extensions-bindings-cosmos/samples/cosmos_samples_containerproxy/local.settings.json
Outdated
Show resolved
Hide resolved
azurefunctions-extensions-bindings-cosmos/samples/cosmos_samples_cosmosclient/function_app.py
Outdated
Show resolved
Hide resolved
| ) | ||
| self.assertEqual( | ||
| e.exception.args[0], | ||
| "Storage account connection string NotARealConnectionString does not exist. " |
There was a problem hiding this comment.
This should be Cosmos DB connection string. How did this test pass?
There was a problem hiding this comment.
Same for the below tests
There was a problem hiding this comment.
Right now the structure is to create a new unit test pipeline for each extension, which needs to be done in ADO, so these tests haven't actually been run anywhere yet. The idea was to maintain separation between the extensions, but it's become more of a hassle when adding new extensions.
Might be worth it to combine every unit test pipeline into one single pipeline so we don't run into this issue again
There was a problem hiding this comment.
Yes - some of the current tests won't pass locally as they require specific setup as well before running (i.e. Env vars)
azurefunctions-extensions-bindings-cosmos/azurefunctions/extensions/bindings/cosmos/__init__.py
Outdated
Show resolved
Hide resolved
|
|
||
| class CosmosClientConverter( | ||
| InConverter, | ||
| OutConverter, |
There was a problem hiding this comment.
Is OutConverter needed here?
| USAGE: | ||
| Set the environment variables with your own values before running the | ||
| sample: | ||
| 1) AzureWebJobsStorage - the connection string to your storage account |
There was a problem hiding this comment.
This should be CosmosDBConnection right? The connection string to the cosmosdb resource. Same for all samples
| "Values": { | ||
| "FUNCTIONS_WORKER_RUNTIME": "python", | ||
| "CosmosDBConnection": "" | ||
| } |
There was a problem hiding this comment.
(I could be wrong) I think AzureWebJobsStorage is a required setting -- we can add it but set it to LocalDev. Same for all samples
Allow customers to bind to CosmosClient, ContainerProxy, and DatabaseProxy from the azure-cosmos SDK.