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 renaming placeholders or mapping placeholders to environment variables with a different name #149

Closed
5 of 6 tasks
zimme opened this issue Feb 4, 2022 · 7 comments

Comments

@zimme
Copy link

zimme commented Feb 4, 2022

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]:

  • am interested in building this feature myself
  • am interested in collaborating on building this feature
  • am willing to help testing this feature before it's released
  • am willing to write a test-driven test suite for this feature (before it exists)
  • am a Graphile sponsor ❤️
  • have an active support or consultancy contract with Graphile
@zimme
Copy link
Author

zimme commented Feb 4, 2022

Would #140 allow using environment variables with different names perhaps?

@benjie
Copy link
Member

benjie commented Feb 4, 2022

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

@zimme
Copy link
Author

zimme commented Feb 4, 2022

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
Copy link
Author

zimme commented Feb 4, 2022

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
Copy link
Member

benjie commented Feb 4, 2022

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 👍

@benjie
Copy link
Member

benjie commented Feb 4, 2022

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

@zimme
Copy link
Author

zimme commented Feb 18, 2022

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...

@zimme zimme closed this as completed Feb 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants