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

Support multiple instances #30

Closed
ksmv-7 opened this issue Jun 2, 2023 · 6 comments · Fixed by #31
Closed

Support multiple instances #30

ksmv-7 opened this issue Jun 2, 2023 · 6 comments · Fixed by #31
Assignees
Labels
enhancement New feature or request

Comments

@ksmv-7
Copy link

ksmv-7 commented Jun 2, 2023

Hello,

We are running multiple instances of Neo4j and use them in a NestJs application. We perform our migrations on application bootstrap with the help of Morpheus4j.
Hence, we are wondering if there is a way to apply the migrations to all instances which are loaded in the application - each instance is a separate host.

Thanks in advance,
Kiril

@marianozunino
Copy link
Owner

Kiril, that's something that you should be able to achieve with NestJs itself. I need to think over on how to make the config something that you can inject multiple times into the service. Something like this:

import { Injectable, Module } from '@nestjs/common';
import { MorpheusModule, MorpheusService } from 'morpheus4j';
import { ConfigService, ConfigModule } from '@nestjs/config';

@Injectable()
export class CustomMigrationService {
  constructor(
    private readonly morpheusService: MorpheusService,
    private readonly configService: ConfigService,
  ) {}

  async onApplicationBootstrap() {
    const configs = [
      {
        scheme: this.configService.get('DB1_SCHEME'),
        host: this.configService.get('DB1_HOST'),
        port: this.configService.get('DB1_PORT'),
        username: this.configService.get('DB1_USERNAME'),
        password: this.configService.get('DB1_PASSWORD'),
        migrationsPath: './neo4j/db1/migrations',
      },
      {
        scheme: this.configService.get('DB2_SCHEME'),
        host: this.configService.get('DB2_HOST'),
        port: this.configService.get('DB2_PORT'),
        username: this.configService.get('DB2_USERNAME'),
        password: this.configService.get('DB2_PASSWORD'),
        migrationsPath: './neo4j/db2/migrations',
      },
    ];

    for (const config of configs) {
      await this.morpheusService.runMigrationsFor(config);
    }
  }
}

@Module({
  imports: [
    ConfigModule.forRoot(),
    MorpheusModule,
  ],
  providers: [CustomMigrationService],
})
export class DbModule {}

This way you can define your own service and execute the migrations for each config that you have.

Again, in order to do this I need to do some refactoring, so I'll think it over.

@marianozunino marianozunino self-assigned this Jun 2, 2023
@marianozunino marianozunino added the enhancement New feature or request label Jun 2, 2023
@ksmv-7
Copy link
Author

ksmv-7 commented Jun 3, 2023

Hello again,

I had pretty much the same vision. It would be very neat. Thanks for considering it!

marianozunino added a commit that referenced this issue Jun 5, 2023
@marianozunino
Copy link
Owner

@ksmv-7 Do you mind testing the feature from the linked branch? I exposed the Morpheus service and updated the NestJs example with some comments and an extra example. Let me know how it goes.

@ksmv-7
Copy link
Author

ksmv-7 commented Jun 6, 2023

@marianozunino Hi, I packaged the project and installed it as npm dependency locally. I have this rather ugly code with the two deletes for testing purposes:

Neo4jModule.forRootAsync({
    imports: [MorpheusModule],
    inject: [MorpheusService],
    useFactory: async (morpheusService: MorpheusService): Promise<Neo4jInstance[]> => {
      const neo4jInstanceManager = Neo4jInstancesManager.getInstance();
      const neo4jInstances = await neo4jInstanceManager.getDbInstances();
      const instancesAndDrivers = await createDrivers(neo4jInstances);
      for (const instance of neo4jInstances) {
        delete instance.databaseType
        delete instance.database
        await morpheusService.runMigrationsFor(instance);
      }
      return Object.values(instancesAndDrivers).map((pair, index) => ({
        ...neo4jInstances[index],
        pair,
      }));
    },
  }),
  MorpheusModule.registerAsync({
    inject: [ConfigService],
    useFactory: (configService: ConfigService) => ({
      scheme: configService.get('MORPHEUS_SCHEME'),
      host: configService.get('MORPHEUS_HOST'),
      port: configService.get('MORPHEUS_PORT'),
      username: configService.get('MORPHEUS_USERNAME'),
      password: configService.get('MORPHEUS_PASSWORD'),
      migrationsPath: './neo4j/migrations', // default value
    }),
  }),

I feel like I am at verge of making it work, however I have a problem with the DependencyScanner which is used in the morpheus project, more specifically in the MigrationService, that I am unable to solve.
Here is the stack:

[Nest] 161879  - 06/06/2023, 11:09:41 AM   ERROR [ExceptionHandler] Cannot read properties of undefined (reading 'scanForModules')
TypeError: Cannot read properties of undefined (reading 'scanForModules')
    at LazyModuleLoader.load (/home/kiril-simeonov/Desktop/projects/nest-backend/node_modules/@nestjs/core/injector/lazy-module-loader.js:16:64)
    at processTicksAndRejections (node:internal/process/task_queues:95:5)
    at FsService.getMigrationsPath (/home/kiril-simeonov/Desktop/projects/nest-backend/node_modules/morpheus4j/dist/cli/fs.service.js:62:27)
    at FsService.getFileContent (/home/kiril-simeonov/Desktop/projects/nest-backend/node_modules/morpheus4j/dist/cli/fs.service.js:116:32)
    at MigrationService.isValidChecksum (/home/kiril-simeonov/Desktop/projects/nest-backend/node_modules/morpheus4j/dist/cli/migration.service.js:139:29)
    at MigrationService.validateMigrationsIntegrity (/home/kiril-simeonov/Desktop/projects/nest-backend/node_modules/morpheus4j/dist/cli/migration.service.js:124:19)
    at MigrationService.migrate (/home/kiril-simeonov/Desktop/projects/nest-backend/node_modules/morpheus4j/dist/cli/migration.service.js:66:9)
    at MorpheusService.executeMigrations (/home/kiril-simeonov/Desktop/projects/nest-backend/node_modules/morpheus4j/dist/morpheus/morpheus.service.js:56:13)
    at MorpheusService.runMigrationsFor (/home/kiril-simeonov/Desktop/projects/nest-backend/node_modules/morpheus4j/dist/morpheus/morpheus.service.js:77:9)
    at InstanceWrapper.useFactory [as metatype] (/home/kiril-simeonov/Desktop/projects/nest-backend/src/app.module.ts:66:9)

I am not sure if the issue comes from my setup or something is off at the root. I ll be looking into it more but I decided to share it with you in case it might be helpful.
I ve also posted in the support Discord channel of NestJs

@marianozunino
Copy link
Owner

marianozunino commented Jun 6, 2023

I pushed another commit making the lazy module optional.

Also, your code should look like this:

Neo4jModule.forRootAsync({
    imports: [MorpheusModule],
    inject: [MorpheusService],
    useFactory: async (morpheusService: MorpheusService): Promise<Neo4jInstance[]> => {
      const neo4jInstanceManager = Neo4jInstancesManager.getInstance();
      const neo4jInstances = await neo4jInstanceManager.getDbInstances();
      const instancesAndDrivers = await createDrivers(neo4jInstances);
      for (const instance of neo4jInstances) {
        delete instance.databaseType
        delete instance.database
        await morpheusService.runMigrationsFor(instance);
      }
      return Object.values(instancesAndDrivers).map((pair, index) => ({
        ...neo4jInstances[index],
        pair,
      }));
    },
  })

No need to execute MorpheusModule.registerAsync since you are going to use the MorpheusService by yourself.

@ksmv-7
Copy link
Author

ksmv-7 commented Jun 9, 2023

@marianozunino Hi again, sorry I didn't have the chance to test it again until now.
It is working amazing.
Would it be just possible to change the

public async runMigrationsFor(config: Neo4jConfig): Promise<void> {
    ConfigLoader.validateConfig(config);
    this.logger.debug(
      'Running migrations for config ' + JSON.stringify(config),
    );
    await this.executeMigrations(config);
  }

to

public async runMigrationsFor(config: Neo4jConfig): Promise<void> {
    ConfigLoader.validateConfig(config);
    this.logger.debug(
      'Running migrations for config ' + JSON.stringify(config.database),
    );
    await this.executeMigrations(config);
  }

for security reasons? This way people will know on which database the migrations are executed without logging the password.

Again, thank you very much!

@marianozunino marianozunino linked a pull request Jun 19, 2023 that will close this issue
github-actions bot pushed a commit that referenced this issue Jun 19, 2023
# [3.1.0](v3.0.2...v3.1.0) (2023-06-19)

### Features

* expose MorpheusService [#30](#30) ([469e232](469e232))
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