graphile / migrate

Opinionated SQL-powered productive roll-forward migration tool for PostgreSQL.
MIT License
751 stars 58 forks source link

Support renaming placeholders or mapping placeholders to environment variables with a different name #149

Closed zimme closed 2 years ago

zimme commented 2 years ago

Feature description

I've run into a situation where the previous names for some of the placeholders could be improved to reflect their more specific use case.

It would be nice to be able to rename placeholders. Not sure if this could be done by having a rename command that finds and replaces the placeholders in the migrations and regenerat the hash:es and I believe some way of updating the migration hashes in the database would need to be found.

An alternative approach would be to be able to map placeholders to environment variables with a different name. I see this as a feature that could make sense outside of the scope of this feature request, but it would provide a way to work around not being able to rename the placeholders.

Motivating example

If you ever want to change the name of a placeholder, you would have to first search replaces the placeholder in all previous migrations as well as add allowInvalidHash comments.

In case you just want to change the name of an environment variable you need to keep the old one and keep them in sync.

Breaking changes

If the hashing part of migrate were to be updated to exclude the placeholders from the hash. Then a simple search replace and rename would be possible, but this would probably be a breaking change.

Supporting development

I [tick all that apply]:

zimme commented 2 years ago

Would https://github.com/graphile/migrate/pull/140 allow using environment variables with different names perhaps?

benjie commented 2 years ago

140 just adds ESM support for the gmrc file that already exists, I think?

zimme commented 2 years ago

Riight, so I could already have a .gmrc.js file today and read alternative environment variables and add them into the config that I export. There's just no ESM support... Not sure this is documented...

Still... The rename of placeholders request still stands I guess. Perhaps it's so unusual to want to rename placeholders that it's not worth adding support for?

zimme commented 2 years ago

~It was documented, it's just kinda not super obvious that the config file can be a js file =)~

Scratch that, it's just me that's not good at reading docs it appears...

benjie commented 2 years ago

I hadn't realised, but yes you're right, in the GMRC you can do:

  "placeholders": {
    ":DATABASE_AUTHENTICATOR": "!ENV",
    ":DATABASE_VISITOR": "!ENV"
  },

but in .gmrc.js you could do:

  placeholders: {
    ":DATABASE_AUTHENTICATOR": "!ENV",
    ":DATABASE_VISITOR": process.env.OTHER_THING_HERE
  },

There might be an argument for adding a sugared syntax for this so that you can do it via both equivalently, but we don't really need it and as far as I can recall you're the only person to ask for it so far. I'd advise you just use the gmrc.js for now :+1:

benjie commented 2 years ago

(If we were to do it, I think your "!ENV:OTHER_THING_HERE" suggestion is a solid one.)

zimme commented 2 years ago

Let's close this as I have a solution and using .js for config files is usually something I do and somehow missed that I could do it with graphile migrate as well...