-
-
Notifications
You must be signed in to change notification settings - Fork 699
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
[17.0][OU-ADD] openupgrade_framework: framework patches #4327
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @acpMicrocom
Thanks a lot for working on openupgrade V17.
Could you preserve git history, cherry picking previous commit present in V16 ?
- it respects authors and history
- it makes easier the diff review between 16.0 and 17.0
Thanks !
/ocabot migration openupgrade_framework
Ok Thanks ! |
If we don't include it, /jsonrpc route doesn't work.
If you have a module in previous versions that adds data on a model, and such model is not loaded in the registry in current version because the module is absent in it, you can't uninstall such module, getting this error: File "odoo/odoo/addons/base/models/ir_model.py", line 1945, in _module_data_uninstall delete(self.env[model].browse(item[1] for item in items)) File "odoo/odoo/api.py", line 463, in __getitem__ return self.registry[model_name]._browse(self, (), ()) File "odoo/odoo/modules/registry.py", line 177, in __getitem__ return self.models[model_name] KeyError: 'model' With this patch, data cleanup of such model is skipped and there's no crash.
…ore ; add links to the new OpenUpgrade website
If corresponding field is None, we need to avoid the "AttributeError: 'NoneType' object has no attribute error.
…rg validate as it is not anymore in odoo 15
…e useless after the input argument validate has been removed
and after the migration
ff25457
to
61ac302
Compare
61ac302
to
e104cf7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks !
@legalsylvain have you checked that each patch method is still the same? It's for not doing 2 times the same work. |
I just check that the function still exists, with the same arguments list. |
@api.model | ||
def _process_end(self, modules): | ||
"""Don't warn about upgrade conventions from Odoo | ||
('fields should be explicitly removed by an upgrade script') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Warning was removed from Odoo in odoo/odoo#56657
This patch can be removed here.
def _get_tests_modules(mod): | ||
""" | ||
Make odoo load tests from openupgrade_scripts/scripts/$module/$version/tests | ||
instead of the standard location |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like https://github.com/odoo/odoo/blob/8c62f4a/odoo/tests/loader.py#L38-L57 makes it possible to load tests from upgrade paths, which is also the aim of this patch. Do you agree @hbrunn?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Played around with this a bit, and I got it to load a test from the --upgrade-path
(subpath base/tests/test_migration_test.py
). Because it has a module path in odoo.upgrade
instead of odoo.addons
it does not get assigned the expected attributes in https://github.com/odoo/odoo/blob/aa84ccf/odoo/tests/common.py#L222-L227. The attributes therefore have to be assigned manually:
from odoo.tests import tagged, TransactionCase
@tagged("at_install", "openupgrade")
class TestMigrationTest(TransactionCase):
def test_migration_test(self):
self.assertEqual(1, 0)
TestMigrationTest.test_module = "base"
TestMigrationTest.test_class = "TestMigrationTest"
TestMigrationTest.test_sequence = 0
After that, the test runs fine and can be selected with test tags base:TestMigrationTest
or openupgrade
.
Personally, I think that is acceptable given that tests are still very much underused and not for the faint of heart at this point.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
agreed the patch becomes obsolete in v17. We could provide a base class for running openupgrade tests, but that's for another PR
@@ -0,0 +1,4 @@ | |||
Many developers have contributed to the OpenUpgrade framework in its | |||
previous incarnation. Their original contributions may no longer needed, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/may no longer needed/may no longer be needed/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hi @rvalyi. I think you can use the "propose change" github feature. It is more readable than the regex syntax ! (and can be approved if desired).
thanks !
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
are you sure it can be used? It adds extra commits noise that many dislike. I don't see the feature used anymore in the OCA. Can you confirm it's easy enough to remove the commit noise vs doing it this way?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't use the github feature for committing myself, because indeed it creates extra commits I'd have to squash again. But still I appreciate reviewers using the feature, because that makes it perfectly clear what they mean.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But still I appreciate reviewers using the feature, because that makes it perfectly clear what they mean.
👍 That is my point. Regex expression is not very easy to read.
I don't use the github feature for committing myself, because indeed it creates extra commits I'd have to squash again
Matter of taste. Once the change proposal is done you can :
- merge then squash
- or make changes locally and push
@iTecan There are no migration scripts yet for OpenUpgrade 17. These are only the supporting framework methods. |
@acpMicrocom The pandoc-3.1.12.1-1-amd64.deb is an artifact of pre-commit initialization. Please remove it from this PR. Can you follow-up the other comments? |
@StefanRijnhart would you like to supersede this PR? |
Superseded by #4427 |
-This commit remove unecessary patch -Check if test exist then execute it in the test.yml -Remove the test loader from patch, detail at OCA#4327 (comment)
-This commit remove unecessary patch -Check if test exist then execute it in the test.yml -Remove the test loader from patch, detail at OCA#4327 (comment)
parent c973596 author 张飞虎 <feihu.zhang@yumtown.com.cn> 1711431231 +0000 committer 张飞虎 <feihu.zhang@yumtown.com.cn> 1717058855 +0000 # This is a combination of 2 commits. # This is the 1st commit message: parent c973596 author 张飞虎 <feihu.zhang@yumtown.com.cn> 1711431231 +0000 committer 张飞虎 <feihu.zhang@yumtown.com.cn> 1717058818 +0000 添加openupgrade_framework代码OCA#4327 增加upgrade_anaylysis_work.txt init base migration script 初步完成base模块的升级脚本 初步添加account模块升级脚本 修复文件名错误 BUG Fix 增加mail模块升级脚本 完成mail升级脚本 初步添加hr升级脚本 完成hr模块升级脚本 # This is the commit message OCA#2: 完成account模块升级脚本 # This is the commit message OCA#3: 更新模块清单 # This is the commit message OCA#4: 添加sale_management升级脚本 # This is the commit message OCA#5: 修复升级问题 # This is the commit message OCA#6: 完善project模块升级脚本 # This is the commit message OCA#7: 增加一堆模块no_update的处理 # This is the commit message OCA#8: BUG Fix # This is the commit message OCA#9: 暂时移除delivery升级脚本 # This is the commit message OCA#10: 改进项目升级脚本 # This is the commit message OCA#11: 添加针对hr_responsible_id字段的处理 # This is the commit message OCA#12: 增加auth_signup升级脚本 # This is the commit message OCA#13: 修复mail_alias上的mail_channel数据
parent c973596 author 张飞虎 <feihu.zhang@yumtown.com.cn> 1711431231 +0000 committer 张飞虎 <feihu.zhang@yumtown.com.cn> 1717058855 +0000 parent c973596 author 张飞虎 <feihu.zhang@yumtown.com.cn> 1711431231 +0000 committer 张飞虎 <feihu.zhang@yumtown.com.cn> 1717058818 +0000 添加openupgrade_framework代码OCA#4327 增加upgrade_anaylysis_work.txt init base migration script 初步完成base模块的升级脚本 初步添加account模块升级脚本 修复文件名错误 BUG Fix 增加mail模块升级脚本 完成mail升级脚本 初步添加hr升级脚本 完成hr模块升级脚本 完成account模块升级脚本 更新模块清单 添加sale_management升级脚本 修复升级问题 完善project模块升级脚本 增加一堆模块no_update的处理 BUG Fix 暂时移除delivery升级脚本 改进项目升级脚本 添加针对hr_responsible_id字段的处理 增加auth_signup升级脚本 修复mail_alias上的mail_channel数据
parent c973596 author 张飞虎 <feihu.zhang@yumtown.com.cn> 1711431231 +0000 committer 张飞虎 <feihu.zhang@yumtown.com.cn> 1717058855 +0000 parent c973596 author 张飞虎 <feihu.zhang@yumtown.com.cn> 1711431231 +0000 committer 张飞虎 <feihu.zhang@yumtown.com.cn> 1717058818 +0000 添加openupgrade_framework代码OCA#4327 增加upgrade_anaylysis_work.txt init base migration script 初步完成base模块的升级脚本 初步添加account模块升级脚本 修复文件名错误 BUG Fix 增加mail模块升级脚本 完成mail升级脚本 初步添加hr升级脚本 完成hr模块升级脚本 完成account模块升级脚本 更新模块清单 添加sale_management升级脚本 修复升级问题 完善project模块升级脚本 增加一堆模块no_update的处理 BUG Fix 暂时移除delivery升级脚本 改进项目升级脚本 添加针对hr_responsible_id字段的处理 增加auth_signup升级脚本 修复mail_alias上的mail_channel数据
parent c973596 author 张飞虎 <feihu.zhang@yumtown.com.cn> 1711431231 +0000 committer 张飞虎 <feihu.zhang@yumtown.com.cn> 1717058855 +0000 parent c973596 author 张飞虎 <feihu.zhang@yumtown.com.cn> 1711431231 +0000 committer 张飞虎 <feihu.zhang@yumtown.com.cn> 1717058818 +0000 添加openupgrade_framework代码OCA#4327 增加upgrade_anaylysis_work.txt init base migration script 初步完成base模块的升级脚本 初步添加account模块升级脚本 修复文件名错误 BUG Fix 增加mail模块升级脚本 完成mail升级脚本 初步添加hr升级脚本 完成hr模块升级脚本 完成account模块升级脚本 更新模块清单 添加sale_management升级脚本 修复升级问题 完善project模块升级脚本 增加一堆模块no_update的处理 BUG Fix 暂时移除delivery升级脚本 改进项目升级脚本 添加针对hr_responsible_id字段的处理 增加auth_signup升级脚本 修复mail_alias上的mail_channel数据
parent c973596 author 张飞虎 <feihu.zhang@yumtown.com.cn> 1711431231 +0000 committer 张飞虎 <feihu.zhang@yumtown.com.cn> 1717058855 +0000 parent c973596 author 张飞虎 <feihu.zhang@yumtown.com.cn> 1711431231 +0000 committer 张飞虎 <feihu.zhang@yumtown.com.cn> 1717058818 +0000 添加openupgrade_framework代码OCA#4327 增加upgrade_anaylysis_work.txt init base migration script 初步完成base模块的升级脚本 初步添加account模块升级脚本 修复文件名错误 BUG Fix 增加mail模块升级脚本 完成mail升级脚本 初步添加hr升级脚本 完成hr模块升级脚本 完成account模块升级脚本 更新模块清单 添加sale_management升级脚本 修复升级问题 完善project模块升级脚本 增加一堆模块no_update的处理 BUG Fix 暂时移除delivery升级脚本 改进项目升级脚本 添加针对hr_responsible_id字段的处理 增加auth_signup升级脚本 修复mail_alias上的mail_channel数据 添加openupgrade_framework代码OCA#4327 parent c973596 author 张飞虎 <feihu.zhang@yumtown.com.cn> 1711431231 +0000 committer 张飞虎 <feihu.zhang@yumtown.com.cn> 1717058855 +0000 parent c973596 author 张飞虎 <feihu.zhang@yumtown.com.cn> 1711431231 +0000 committer 张飞虎 <feihu.zhang@yumtown.com.cn> 1717058818 +0000 添加openupgrade_framework代码OCA#4327 增加upgrade_anaylysis_work.txt init base migration script 初步完成base模块的升级脚本 初步添加account模块升级脚本 修复文件名错误 BUG Fix 增加mail模块升级脚本 完成mail升级脚本 初步添加hr升级脚本 完成hr模块升级脚本 完成account模块升级脚本 更新模块清单 添加sale_management升级脚本 修复升级问题 完善project模块升级脚本 增加一堆模块no_update的处理 BUG Fix 暂时移除delivery升级脚本 改进项目升级脚本 添加针对hr_responsible_id字段的处理 增加auth_signup升级脚本 修复mail_alias上的mail_channel数据
parent c973596 author 张飞虎 <feihu.zhang@yumtown.com.cn> 1711431231 +0000 committer 张飞虎 <feihu.zhang@yumtown.com.cn> 1717058855 +0000 parent c973596 author 张飞虎 <feihu.zhang@yumtown.com.cn> 1711431231 +0000 committer 张飞虎 <feihu.zhang@yumtown.com.cn> 1717058818 +0000 添加openupgrade_framework代码OCA#4327 增加upgrade_anaylysis_work.txt init base migration script 初步完成base模块的升级脚本 初步添加account模块升级脚本 修复文件名错误 BUG Fix 增加mail模块升级脚本 完成mail升级脚本 初步添加hr升级脚本 完成hr模块升级脚本 完成account模块升级脚本 更新模块清单 添加sale_management升级脚本 修复升级问题 完善project模块升级脚本 增加一堆模块no_update的处理 BUG Fix 暂时移除delivery升级脚本 改进项目升级脚本 添加针对hr_responsible_id字段的处理 增加auth_signup升级脚本 修复mail_alias上的mail_channel数据 添加openupgrade_framework代码OCA#4327 parent c973596 author 张飞虎 <feihu.zhang@yumtown.com.cn> 1711431231 +0000 committer 张飞虎 <feihu.zhang@yumtown.com.cn> 1717058855 +0000 parent c973596 author 张飞虎 <feihu.zhang@yumtown.com.cn> 1711431231 +0000 committer 张飞虎 <feihu.zhang@yumtown.com.cn> 1717058818 +0000 添加openupgrade_framework代码OCA#4327 增加upgrade_anaylysis_work.txt init base migration script 初步完成base模块的升级脚本 初步添加account模块升级脚本 修复文件名错误 BUG Fix 增加mail模块升级脚本 完成mail升级脚本 初步添加hr升级脚本 完成hr模块升级脚本 完成account模块升级脚本 更新模块清单 添加sale_management升级脚本 修复升级问题 完善project模块升级脚本 增加一堆模块no_update的处理 BUG Fix 暂时移除delivery升级脚本 改进项目升级脚本 添加针对hr_responsible_id字段的处理 增加auth_signup升级脚本 修复mail_alias上的mail_channel数据
-This commit remove unecessary patch -Check if test exist then execute it in the test.yml -Remove the test loader from patch, detail at #4327 (comment)
-This commit remove unecessary patch -Check if test exist then execute it in the test.yml -Remove the test loader from patch, detail at OCA#4327 (comment)
-This commit remove unecessary patch -Check if test exist then execute it in the test.yml -Remove the test loader from patch, detail at OCA#4327 (comment)
No description provided.