Skip to content

Conversation

sebbi08
Copy link
Contributor

@sebbi08 sebbi08 commented Jan 10, 2025

Readiness checklist

  • I added/updated tests.
  • I ensured that the PR title is good enough for the changelog.
  • I labeled the PR.
  • I self-reviewed the PR.

@sebbi08 sebbi08 added the enhancement New feature or request label Jan 10, 2025
Comment on lines 65 to 102
protected async createTransport(): Promise<void> {
if (this.transport) {
return;
}
if (!this.connectorConfig) throw new Error("Connector config not initialized");
const eventBus = new EventEmitter2EventBus(() => {
// ignore errors
});
const logger = new NodeLoggerFactory(this.connectorConfig.logging);
this.databaseConnection = await BaseCommand.createDBConnection(this.connectorConfig);

this.transport = new Transport(this.databaseConnection, { ...this.connectorConfig.transportLibrary, supportedIdentityVersion: 1 }, eventBus, logger);
await this.transport.init();
}

public static async createDBConnection(runtimeConfig: ConnectorRuntimeConfig): Promise<IDatabaseConnection> {
if (runtimeConfig.database.driver === "lokijs") {
if (!runtimeConfig.debug) throw new Error("LokiJS is only available in debug mode.");

const folder = runtimeConfig.database.folder;
if (!folder) throw new Error("No folder provided for LokiJS database.");

return new LokiJsConnection(folder, undefined, { autoload: true, autosave: true, persistenceMethod: "fs" });
}

if (!runtimeConfig.database.connectionString) {
throw new Error(`No database connection string provided. See ${DocumentationLink.operate__configuration("database")} on how to configure the database connection.`);
}

const mongodbConnection = new MongoDbConnection(runtimeConfig.database.connectionString);

try {
await mongodbConnection.connect();
} catch (e) {
throw new Error(`Could not connect to the configured database. Try to check the connection string and the database status. Root error: ${e}`);
}
return mongodbConnection;
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jkoenig134 I currently don't like this as it duplicates the DB creation code from the ConnectorRuntime.
Best would be to export the DB creation out of the ConnectorRuntime, but don't know where it fits best

Copy link
Member

Choose a reason for hiding this comment

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

WTB just starting the runtime with allowIdentityCreation set to true in this command? Then the runtime does the rest.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yea, but we then could not distinguish if the identity already existed or if it was just created. Don't know if we need that, but yea, that would make it way easier.

Copy link
Member

Choose a reason for hiding this comment

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

I'll think about it. I like the idea to throw an error if the identity already exists..

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I now build it so that the already thrown exception is used.

I don't think that throwing an exception when the identity already exists is a good idea because we would need an exception for our test, or rewrite all that "reuse" an existing identity

Copy link

codecov bot commented Jan 10, 2025

Codecov Report

Attention: Patch coverage is 65.55556% with 31 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/cli/BaseCommand.ts 71.69% 15 Missing ⚠️
src/cli/commands/identity/init.ts 55.88% 15 Missing ⚠️
src/index.ts 50.00% 1 Missing ⚠️
Files with missing lines Coverage Δ
src/cli/commands/index.ts 100.00% <100.00%> (ø)
src/index.ts 76.31% <50.00%> (ø)
src/cli/BaseCommand.ts 65.78% <71.69%> (ø)
src/cli/commands/identity/init.ts 55.88% <55.88%> (ø)

@jkoenig134 jkoenig134 added the wip Work in Progress (blocks mergify from auto update the branch) label Jan 29, 2025
@sebbi08 sebbi08 marked this pull request as ready for review July 3, 2025 09:04
@jkoenig134 jkoenig134 marked this pull request as draft July 3, 2025 15:48
@jkoenig134
Copy link
Member

I put this back to draft as it will not land in v7

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request wip Work in Progress (blocks mergify from auto update the branch)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants