-
Notifications
You must be signed in to change notification settings - Fork 180
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
feat: support migrations in subdirectories #1226
Conversation
I've tried to use this branch in main project I work on (the one that has nearly 500 migrations), and I'd actually like to change my approach a bit. I'm curious what you think: In this PR so far I've defined the name of the migration as the path to the migration. So a migration file like However, since my goal is to migrate an existing code base, this makes things very awkward. If we rename existing migrations, it is very easy to end up in a case where we try to run the migrations from scratch because the names are different now. I can get it to work by manually editing the database to update the migration names, but I'm not very comfortable doing that. Instead, what do you think about the name of the migration being only the last part of the path? So the file The main concern I have with this is that if there are migrations in different paths but with the same basename (like |
IMO e.g. |
Do you have any ideas about how to transition existing sets of migrations to use a new structure? |
No, but IMO also not in the scope of node-pg-migrate. |
In that case I think this PR is ready to be reviewed. I'll have to figure out how to transition my project, but if that's out of scope, then that isn't a concern for this PR. |
ok, will have a look later, but right now I assume my weekend will be very full, even first half of next week is full. maybe @osdiab could have a look in beforehand |
To make sure I understand before jumping into the review is the intention that
Is this right? |
Not quite. Points 1 and 3 are accurate, but not 2. Although I talked about having subfolders not affect order, that's not what I did originally. The code implemented here does take into account the sub folder names when determining order. I'm on the fence about if sub folders should or should not affect order. Shinigami92 seems pretty confident that they should, and I'm happy to defer to that. |
Ok, I’m cool with that and will take a look soon! |
Oh I see, I think I need to come up with some acceptance criteria, so we can align each other. @osdiab while reviewing the code, please make familiar also with your own thoughts not only about that the code is ok, but also
sadly I do not have much time right now, so I will come back (potentially also asking ChatGPT a bit) and define some acceptance criteria for this feature request |
to help things along:
|
I really like to opt-in idea! Lets do this and by default it will be turned off, so it behaves just right now. results in:
so in other words, it will take the full path, and sort by It is potentially also open to define another options to pass a custom sort, but this would be its own PR I assume, because it is another feature request. @mythmon What do you think? Are you fine with this? |
That all sounds fine to me. It will probably be a few days until I can modify this PR to make those changes. Feel free to modify this branch as you see fit if you get to it before me, however. |
In your example
shouldn't |
I thought about using it this way, because it is the structure you would also have in File Explorer or your IDE (e.g. VSCode) |
That would require something a bit more sophisticated than just I think that as a user I'd prefer directories to be sorted the same as files instead of always coming first though. I'm thinking of this use case:
That would be hard to do with the sorting scheme out suggest. |
But that would enforce a specific naming again. I need to think a bit about what the best sorting strategy might be 🤔 |
I don't think it enforces a specific naming pattern. It's like you said:
That would work with dates, numbers, or even just non-numeric. If you want all the directories to come first you could prefix them all with something alphabetically-low. If you want them to come last you could do that too. On the other hand, if directories are defined as always coming before peer files, then you can't override the directory ordering with a file name. |
I thought now a bit and I tend to agree... Neither way, the user could add files somewhere in between and so crash the migration order of an existing db. So, maybe we should lay more focus on good documentation and suggestions. Maybe later on we could think about of repair/regenerate strategies. Or updating strategies to allow a patch in between. Like a "I know what I'm doing, and I just want to patch and now my DBs are all green with that patch"-mode. So for now, I think you are one of the requesters / users that want this subdirectory feature, and I should kinda trust you what would potentially be best. |
have you considered using glob instead? I would make the current options vs glob config mutually exclusive to not break backwards-compatibility. also willing to contribute a PR. |
you can try, give it a go |
@Shinigami92 I was waiting for more feedback from you about this, since you seem to agree with my assessment above. Is there more you wanted me to do here? Sorry if I've missed something. |
My last feedback was in #1226 (comment) But I also see that the Pipeline is red and the branch is not up-to-date So right now it is your turn to move on 😺 |
@mythmon I think this library shouldn't implement generating migration files in a particular folder structure in order to adhere to workflows of individual users. I'd rather recommend to execute the the use-case of subdirectories should also be covered by the glob functionality proposed in #1274 , do you agree? |
The glob pattern support landed in v7.7.0 |
I think the glob support from #1274 would handle most of what I wanted from this PR. Thanks for implementing that @benkroeger! I think the main other problem that I'll have in the future is changing the file structure of migrations. Since the name of the migration would change if a file is moved, it means that we can't re-organize migrations after the fact without a complicated and manual process. Neither PR has a solution to that if I'm reading things right. This will be helpful if we ever set up a new migration system though. |
Adds support for migrations stored in subdirectories, which can help organize large groups of migrations. Also adds support for creating migrations in year-prefixed directories.
Fixes #932