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

FIX: Create readonly functions during backup #7981

Merged
merged 2 commits into from
Aug 9, 2019

Conversation

gschlager
Copy link
Member

Temporarily recreate already dropped functions in the discourse_functions schema in order to allow restoring of backups which still reference dropped functions.

@discoursebot
Copy link

You've signed the CLA, gschlager. Thank you! This pull request is ready for review.

@eviltrout
Copy link
Contributor

Does this mean in the future we must create the constants DROPPED_TABLES, DROPPED_COLUMNS in order for things to work properly?

@gschlager
Copy link
Member Author

Yes, exactly. I couldn't think of another way of knowing which functions to create. Now that you mention it I could add a spec that ensures that post migrations contain the right constants when ColumnDropper or TableDropper are used. That should prevent mistakes in most cases.

Temporarily recreate already dropped functions in the discourse_functions schema in order to allow restoring of backups which still reference dropped functions.
@gschlager
Copy link
Member Author

I went ahead and added specs to check the post migrations.

@eviltrout
Copy link
Contributor

This is definitely better, but I wonder if there's a way we could query directly as a result of the ColumnDropper calls? Could those calls set some class variables in a certain context and then allow us to check those variables?

@gschlager
Copy link
Member Author

That's actually a good idea! The migrations aren't running during the restore, but I guess I could load them as I do right now and execute the up or change method. Running them in a block that makes the execute_drop calls only record the table and column names should be possible. I can give it another go.

@gschlager
Copy link
Member Author

Actually, that won't work. There's other code in some of those migrations and I can't blindly execute all post migrations. :/

@eviltrout
Copy link
Contributor

Ah too bad. Was worth a look though!

@gschlager gschlager merged commit 74d78e3 into discourse:master Aug 9, 2019
@gschlager gschlager deleted the discourse_functions branch August 9, 2019 09:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants