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

[#172332578] singleton cosmosdb client #47

Merged
merged 1 commit into from
Apr 18, 2020

Conversation

gunzip
Copy link
Contributor

@gunzip gunzip commented Apr 18, 2020

No description provided.

@pagopa-github-bot
Copy link
Contributor

Warnings
⚠️

Please include a description of your PR changes.

Affected stories

  • ⚙️ #172332578: Usare un unico client CosmosDB condiviso tra le function

Generated by 🚫 dangerJS

@gunzip gunzip merged commit 22345bb into master Apr 18, 2020
@gunzip gunzip deleted the 172332578-cosmosdb-singleton branch April 18, 2020 22:27
// tslint:disable-next-line: no-let
let instance: DocumentDBClient;

export function getDocumentClient(
Copy link
Contributor

Choose a reason for hiding this comment

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

This is actually not a good implementation - it works in our case since the params don't change, but it's misleading and can cause bugs in the future - instance will be created with the cosmosDbUri and cosmosDbKey provided in the first call - these parameters will be ignored on subsequent calls - so say somebody tries to call this function to get the client for multiple cosmosdb URIs, it will not work as expected.

The problem is that a singleton is supposed to receive the creation params once (on constructor).

You have two options:

  1. (simpler, preferred) instantiate a DocumentDBClient right away in this file and export it, i.e.
const cosmosDbUri = getRequiredStringEnv("COSMOSDB_URI");
const masterKey = getRequiredStringEnv("COSMOSDB_KEY");

export const documentDbClient =  new DocumentDBClient(cosmosDbUri, { masterKey }));
  1. (little more complex but doesn't add much) make it a singleton class, and do as in (1)

  2. (more complex, not needed since we have only one URL) make getDocumentClient keep a map of cosmosDbUriDocumentDBClient and make it instantiate/return the instance associated to the URL

Copy link
Contributor Author

Choose a reason for hiding this comment

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

you're right I'm fixing it here and for io-functions-app

@cloudify
Copy link
Contributor

@gunzip sorry I was late in reviewing you code

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.

3 participants