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

install takes the old version, not the version being installed #7

Merged
merged 1 commit into from
May 31, 2024

Conversation

zachdaniel
Copy link
Contributor

We fixed a bug in ash_postgres where it was doing the wrong thing here, so it makes sense that this broke, but the solution is to fix downstream extensions.

We fixed a bug in `ash_postgres` where it was doing the wrong thing here, so it makes sense that this broke, but the solution is to fix downstream extensions.
@moissela
Copy link
Member

I think is more clear that the install function argument indicates the version to generate the migration of, rather than the previous version from which you are coming.

My original inspiration for this come from here.

Currently, AshPostgres is not calling every intermediate version between the previously installed one and the latest available one, so it is still install function's responsibility to generating a valid migration even if some intermediate versions are skipped.

What do you think? Maybe there are other motivations around this change that I'm not aware of?

In relation to this, I open a PR to AshPostgre here which restore this behaviour correctly.

@zachdaniel
Copy link
Contributor Author

I've addressed the PR you mentioned just now. It's intentional that we provide the old version of the extension that the user is on. As you make changes, you're meant to update the relevant migration functions to provide a minimal migration path from wherever the user was before. Without it, you can't know what changes must be made.

@moissela
Copy link
Member

Okay, now I understand what you mean!

The current version of an extension correctly defines the current state (identified by the attribute latest_version) and exposes a specific install method to be able to reach the current state from any previous version in the most optimized way possible.
If the extension had never been used in the user's project before, then AshPostgres calls install(0) indicating that it wants to obtain the path from nothing to the current version.

Got it, thanks for your time!

@moissela moissela merged commit 50c0c97 into zoonect-oss:main May 31, 2024
11 of 13 checks passed
@zachdaniel
Copy link
Contributor Author

Yes, exactly 🥳

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

Successfully merging this pull request may close these issues.

2 participants