marianozunino / morpheus

Morpheus is a modern, open-source database migration tool for Neo4j. It is designed to be a simple, intuitive tool for managing database migrations. The project is inspired by Michael Simons' tool for Java.
MIT License
19 stars 4 forks source link

Select database in config #33

Closed nickeday closed 1 year ago

nickeday commented 1 year ago

Hey, is there any way to specify which database you will be working with ahead of time, perhaps as part of the config? At the moment I tried using :use statements in my migration files, but when I run 'yarn morpheus migrate' it looks like it's automatically trying to make a change in the 'neo4j' database, for which I don't have permissions. I end up with the following error

$ morpheus migrate [Morpheus] Creating new node label on database 'neo4j' is not allowed for user '{excised}' with roles [{excised}]. See GRANT CREATE NEW NODE LABEL ON DATABASEneo4j... error Command failed with exit code 1.

marianozunino commented 1 year ago

That would be simple to add. But I wonder if it's worth to refactor in order so I don't handle the connection no more and let the consumer pass in the connection that's going to be used for the migrations. That way you are 100% under control about when to start/end and how to configure such connection. Also, that only would work for people using the this as a library and not in the cli :thinking:

nickeday commented 1 year ago

Firstly, thank you for replying so quickly it's much appreciated!

Yes, I'm a CLI user so the refactor wouldn't solve my issue unfortunately. Would adding a database parameter to the config potentially cause any issues? I guess it'd be like setting the 'default' database that morpheus uses to make it's behind the scenes changes and then the user is free to use :use commands in their migrations after that.

marianozunino commented 1 year ago

@nickeday I created a branch, can you pull it and see if that works?

Setps:

git clone --branch feature/database-in-config git@github.com:marianozunino/morpheus
cd morpheus
yarn && yarn build

# now you use the script generated under dist as:
node dist/main.js init|create|migrate|etc

I did a small test but when using the community edition it doesn't matter which database you specify in the connection URI, it will always use the one that you have in your neo4j config:

dbms.default_database=neo4j #this value is the default one

Is fair to assume that you are using the enterprise edition? if so, then this https://github.com/marianozunino/morpheus/issues/5 issue is related to that. In that case I would advice using Michael Simons` CLI App

nickeday commented 1 year ago

Thank you for creating the branch. Hmm yes we're on enterprise edition with the details

[ { name: 'Neo4j Kernel', version: '5.10.0', edition: 'enterprise' } ]

When using your branch, I can see my specified db being picked up from the config now and used in the connectionUrl, but it does seem to just fall back to using the 'neo4j' database as you say. I had a dig around in the code and can see the cypher-query-builder package is using neo4j-driver package under the hood. I think the problem is that with neo4j-driver, as far as I'm aware, the only way to specify the database you want a query to run on is when you call neo4j.driver.session({ database: ... }). Unfortunately it looks like cypher-query-builder only ever just calls driver.session() without any config, and as the driver is a protected property, there's no way I can see to get around it.

nickeday commented 1 year ago

In fact scratch that, you can work around it by overwriting the cypher-query-builder Connection.session function and forcing it to use dbConfig.database. So an updated morpheus getDatabaseConnection function might look like below. I'm obviously not a fan of overwriting lib functions generally! Though perhaps for cypher-query-builder it might be ok as it's unlikely there'll be another release of it given the last was 3 years ago, so keeping this patch in line with new releases might not be a burden.

export const getDatabaseConnection = async (
  dbConfig: Neo4jConfig = ConfigLoader.getConfig(),
) => {
  let connectionUrl = `${dbConfig.scheme}://${dbConfig.host}:${dbConfig.port}`;

  const connection = new Connection(connectionUrl, {
    username: dbConfig.username,
    password: dbConfig.password,
  });

  if (dbConfig.database) {
    connection.session = function() {
      if (this.open) {
        return this.driver.session({ database: dbConfig.database});
      }
      return null;
    }
  }

  await connection.query().raw('RETURN 1').run();
  return connection;
};

I used this and can see running the migrate command from the CLI has created the __Neo4jMigration BASELINE node in my specified database.

marianozunino commented 1 year ago

@nickeday Mind to create a PR with such changes? Don't worry about the pipeline failing, that's just because my remote Aura DB is suspended.

nickeday commented 1 year ago

PR now open at https://github.com/marianozunino/morpheus/pull/34

nickeday commented 1 year ago

just a quick note I'm going to be offline from now until coming Tuesday so I won't be able to test anything out or respond til then. Thanks again!

marianozunino commented 1 year ago

Thanks @nickeday. Whenever you can just go ahead an use the new published version.