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

[ADD] #9500 Added the migration script to waft. #29

Open
wants to merge 113 commits into
base: master
Choose a base branch
from

Conversation

ddejong-therp
Copy link
Contributor

I've added the migration script to waftlib. Not 100% sure if the placement of files/directories is in line with the conventions. But that can always be changed of course.

Another thing I still like to change, is the fact that currently the builds of the migration are build when you use the -r switch in the migration script. However, maybe it is more nice if that is included into the ./build script. Maybe the build script can detect whether a migration is enabled in .env-*, and if so, set up the intermediate migration builds as well.
Perhaps I can still do this once the clouds clear up and I have some time. :)

@@ -1 +1 @@
3.10.6
3.7.12
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ddejong-therp Why this change? Several of our instances are currently happily running on Python 3.10.6 and would be reverted to 3.7.12 on the next run of ./bootstrap, so we'd have to plan that in a weekend and run into all sorts of problems - I'd rather not do that unless there's a good reason

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah that is a good question. I actually don't really know. I think it was because I assumed that it was 3.6 before, so I assumed I changed it to something weird. Must have been a brainfart...

@thomaspaulb
Copy link
Member

@ddejong-therp I also see one conflict, can you rebase?

@thomaspaulb
Copy link
Member

Another thing I still like to change, is the fact that currently the builds of the migration are build when you use the -r switch in the migration script. However, maybe it is more nice if that is included into the ./build script.

Personally I would keep this separate, so that we decrease the risk of improvements in our migration script causing regressions with the ./build command of other Waft instances.

@ddejong-therp
Copy link
Contributor Author

Another thing I still like to change, is the fact that currently the builds of the migration are build when you use the -r switch in the migration script. However, maybe it is more nice if that is included into the ./build script.

Personally I would keep this separate, so that we decrease the risk of improvements in our migration script causing regressions with the ./build command of other Waft instances.

yeah that's fair.

@@ -1011,7 +1011,7 @@ def run_upgrade(version):
else MIGRATION_PATH + '/build-' + version

logfile = os.path.join(WAFT_DIR, 'logfile', instance + '.log')
args = '-u all --stop-after-init --logfile "%s"' % logfile
args = '-u base --stop-after-init --logfile "%s"' % logfile
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ddejong-therp What's the difference?

Copy link
Contributor Author

@ddejong-therp ddejong-therp Feb 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The difference shows up when you are trying to get modules removed using the 'to remove' state.

Right now, the uninstall script only puts the modules to be uninstalled in the 'to remove' state, because if you use the button_immediate_uninstall function, that sometimes only puts the module in the 'to remove' state anyway, which should get uninstalled later. But because I was running it from a shell, there was not future opportunity to get those modules uninstalled.

Another way to trigger the uninstallation of 'to remove' modules, is to run odoo with the upgrade switch (-u). However, if you upgrade the module with the 'to remove' state itself, it will also update the state field back to 'installed', before triggering the code that continues to remove the 'to remove' modules.
When you use -u all, you reset all the modules state back to 'installed', and when you use -u base, you don't, but you still trigger the code that does the uninstallation.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow, ok. I didnt know that.

@ddejong-therp ddejong-therp force-pushed the migration-script branch 7 times, most recently from 99a6ff3 to e992c3e Compare May 29, 2024 10:16
@ddejong-therp ddejong-therp force-pushed the migration-script branch 3 times, most recently from 19da9ee to 8724e5d Compare September 16, 2024 10:37
@ddejong-therp ddejong-therp force-pushed the migration-script branch 5 times, most recently from 6cb23d1 to 3e84fda Compare September 23, 2024 08:14
bin/migrate.py Outdated
cmd_system(os.path.join(build_dir, "bootstrap"))
with open(os.path.join(build_dir, ".env-secret"), "r") as file:
env_secret_template = os.path.join(build_dir, "waftlib/templates/.env-secret")
with open(env_secret_template, "r") as file:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ddejong-therp Do you still run bootstrap somewhere though?

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.

3 participants