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

[FEATURE] In the connector, support MongoDB database names that don't start with "acc-" #22

Closed
blalasaadri opened this issue May 7, 2024 · 4 comments · Fixed by nmshd/connector#152
Labels
enhancement New feature or request

Comments

@blalasaadri
Copy link

Environment
Connector

Is your feature request related to a problem? Please describe.
We deploy our instance of the connector in a cloud environment, where we don't have control over the name assigned to our MongoDB databases. However due to the following line in the initAccount method of ConnectorRuntime.ts, this doesn't work:

const db = await this.transport.createDatabase(`acc-${this.runtimeConfig.database.dbName}`);

As can be seen here, the connector requires the database name to always start with acc-.

Describe the solution you'd like
Rather than always requiring this prefix, it should be possible to fully configure the database name.
For backwards compatibility reasons, I would suggest the following in the ConnectorRuntime.ts:

const db = await this.transport.createDatabase(`${this.runtimeConfig.database.dbNamePrefix ?? 'acc-'}${this.runtimeConfig.database.dbName}`);

This means that an optional dbNamePrefix can be passed in the configuration. If no such parameter is given, the behaviour remains unchanged.
In our case, the relevant part of the config looks like this:

{
 "database": {
    "dbNamePrefix": ""
  }
}

Describe alternatives you've considered
Currently we're running patch files on four files to enable the desired behaviour. I have attached these patch files as part of this ticket; in our own Dockerfile (which is a modified copy of the Connector Dockerfile we run the patches as follows:

# basic setup, of the builder, retrieve the code (ommited for brevity)

COPY build/ConnectorRuntime.ts.patch ConnectorRuntime.ts.patch
RUN patch src/ConnectorRuntime.ts < ConnectorRuntime.ts.patch
COPY build/ConnectorRuntimeConfig.ts.patch ConnectorRuntimeConfig.ts.patch
RUN patch src/ConnectorRuntimeConfig.ts < ConnectorRuntimeConfig.ts.patch
COPY build/connectorConfig.json.patch connectorConfig.json.patch
RUN patch src/jsonSchemas/connectorConfig.json < connectorConfig.json.patch
COPY build/connectorConfig.ts.patch connectorConfig.ts.patch
RUN patch src/jsonSchemas/connectorConfig.ts < connectorConfig.ts.patch

RUN npm ci

RUN npm run build

RUN .ci/writeBuildInformation.sh

If this feature were to be implemented, we could use the provided Docker image instead.

Our patch files
connectorConfig.json.patch (for the src/jsonSchemas/connectorConfig.json)
connectorConfig.ts.patch (for the src/jsonSchemas/connectorConfig.ts)
ConnectorRuntime.ts.patch (for the src/ConnectorRuntime.ts)
ConnectorRuntimeConfig.ts.patch (for the src/ConnectorRuntimeConfig.ts)

@jkoenig134
Copy link
Member

Hi @blalasaadri thanks for the detailed feature request.

I have created a PR that should fix your problem.
When this is merged you can simply add the following to your config:

{
  "database": {
    "dbNamePrefix": ""
  }
}

@jkoenig134
Copy link
Member

@blalasaadri connector version 3.10.1 is now available, feel free to try it out and report back!

@blalasaadri
Copy link
Author

Thank you @jkoenig134, that was very fast and it works perfectly. 🙂

@jkoenig134
Copy link
Member

Glad to hear that your issue is fixed!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants