Refactor Migration Tool#4167
Conversation
|
@rohnsha0 Take a look at what I did in here. @mauritsvanrees, @davisagli: Opinions? |
rohnsha0
left a comment
There was a problem hiding this comment.
@ericof please see:-
on setup page:-
2025-04-29 10:56:26,592 ERROR [Zope.SiteErrorLog:35][waitress-2] AttributeError: http://localhost:8080/@sites
Traceback (innermost last):
Module ZPublisher.WSGIPublisher, line 181, in transaction_pubevents
Module ZPublisher.WSGIPublisher, line 390, in publish_module
Module ZPublisher.WSGIPublisher, line 284, in publish
Module ZPublisher.mapply, line 98, in mapply
Module ZPublisher.WSGIPublisher, line 68, in call_object
Module plone.rest.service, line 21, in __call__
Module plone.distribution.services.sites.get, line 46, in render
Module plone.distribution.services.sites.get, line 121, in reply
Module plone.distribution.services.sites.get, line 81, in get_sites
Module plone.distribution.services.sites.get, line 23, in _is_outdated
Module Products.CMFPlone.MigrationTool, line 310, in needUpgrading
Module Products.CMFPlone.MigrationTool, line 252, in getInstanceVersion
AttributeError: 'RequestContainer' object has no attribute 'setup'
on /@@overview-controlpanel
2025-04-29 10:57:57,313 ERROR [Zope.SiteErrorLog:35][waitress-3] TypeError: http://localhost:8080/Plone/@@overview-controlpanel
Traceback (innermost last):
Module ZPublisher.WSGIPublisher, line 181, in transaction_pubevents
Module ZPublisher.WSGIPublisher, line 390, in publish_module
Module ZPublisher.WSGIPublisher, line 284, in publish
Module ZPublisher.mapply, line 98, in mapply
Module ZPublisher.WSGIPublisher, line 68, in call_object
Module Products.CMFPlone.controlpanel.browser.overview, line 35, in __call__
Module Products.Five.browser.pagetemplatefile, line 127, in __call__
Module Products.Five.browser.pagetemplatefile, line 59, in __call__
Module zope.pagetemplate.pagetemplate, line 134, in pt_render
Module Products.PageTemplates.engine, line 368, in __call__
Module z3c.pt.pagetemplate, line 198, in render
Module chameleon.zpt.template, line 328, in render
Module chameleon.template, line 229, in render
Module chameleon.utils, line 20, in raise_with_traceback
Module chameleon.template, line 200, in render
Module 9853bf9687268a7b1899879f481dadac, line 1249, in render
Module eff93f466a1c56c5c3659cf9646e3174, line 903, in render_master
Module 802a2dde8c920121c249f7a13ad5c12d, line 919, in render_master
Module eff93f466a1c56c5c3659cf9646e3174, line 878, in __fill_content
Module 802a2dde8c920121c249f7a13ad5c12d, line 1567, in render_content
Module eff93f466a1c56c5c3659cf9646e3174, line 864, in __fill_main
Module 9853bf9687268a7b1899879f481dadac, line 473, in __fill_prefs_configlet_main
Module zope.tales.expressions, line 324, in __call__
Module Products.PageTemplates.Expressions, line 299, in evaluateBoolean
Module zope.tales.tales, line 768, in evaluate
- Expression: <PathExpr standard:'view/pil'>
- Names:
{'args': (),
'container': <PloneSite at /Plone>,
'context': <PloneSite at /Plone>,
'default': <DEFAULT>,
'here': <PloneSite at /Plone>,
'loop': {'configlet': <Products.PageTemplates.engine.RepeatItem object at 0x7b47d17b2d90>,
'group': <Products.PageTemplates.engine.RepeatItem object at 0x7b47d4484590>},
'nothing': None,
'options': {},
'repeat': <Products.PageTemplates.Expressions.SafeMapping object at 0x7b47d17f15d0>,
'request': <WSGIRequest, URL=http://localhost:8080/Plone/@@overview-controlpanel>,
'root': <Application at >,
'template': <Products.Five.browser.pagetemplatefile.ViewPageTemplateFile object at 0x7b47d481b910>,
'traverse_subpath': [],
'user': <PropertiedUser 'admin'>,
'view': <Products.Five.browser.metaconfigure.OverviewControlPanel object at 0x7b47d1bf42d0>,
'views': <Products.Five.browser.pagetemplatefile.ViewMapper object at 0x7b47d3d7d990>}
Module zope.tales.expressions, line 248, in __call__
Module Products.PageTemplates.Expressions, line 225, in _eval
Module Products.PageTemplates.Expressions, line 155, in render
Module Products.CMFPlone.controlpanel.browser.overview, line 50, in pil
Module plone.memoize.instance, line 53, in memogetter
Module Products.CMFPlone.controlpanel.browser.overview, line 47, in core_versions
Module Products.CMFPlone.MigrationTool, line 329, in coreVersions
Module Products.CMFPlone.MigrationTool, line 45, in _pil_version
TypeError: 'builtin_function_or_method' object does not support item assignment
|
@rohnsha0 Could you, please, try again? |
|
@rohnsha0 sorry for late response, was a bit busy with dev for recyclebin... i tested your fix but |
|
maybe something wrong at my end?! |
|
I will double check why this is failing for you. |
|
I mostly see some code being reshuffled. It looks like it should continue to work, but I don't yet see why this would be better than before. It is hard to see if any accidental omissions or other mistakes have been made. I see a few new versions reported, which is fine. And is the logging stream meant to give more feedback about what happens when running upgrades? BTW, the reporting of the PIL version can probably be skipped, though best not in a bug fix release. It was useful years ago, but now it will always just be Pillow. |
|
I have identified and fixed the issue on kitconcept-core.
I do plan to port the fiz here during Beethoven Sprint
[]s
Érico Andrei
…On Mon, May 26, 2025, 19:11 Maurits van Rees ***@***.***> wrote:
*mauritsvanrees* left a comment (plone/Products.CMFPlone#4167)
<#4167 (comment)>
I mostly see some code being reshuffled. It looks like it should continue
to work, but I don't yet see why this would be better than before. It is
hard to see if any accidental omissions or other mistakes have been made.
I see a few new versions reported, which is fine. And is the logging
stream meant to give more feedback about what happens when running upgrades?
BTW, the reporting of the PIL version can probably be skipped, though best
not in a bug fix release. It was useful years ago, but now it will always
just be Pillow.
—
Reply to this email directly, view it on GitHub
<#4167 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AACKWXXRHFS2I7WLTMUYMHD3ANDKJAVCNFSM6AAAAAB4BSFCGOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDSMJQGI4TEMJQGY>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
@ericof Do you still have updates to make here before I review it? |
|
@davisagli, @mauritsvanrees, I'm going to work on this next week and get it ready for a review. |
|
One problem with the current PR is that this is a branch based on another branch, and not on master, so a bit indirect, and those branches are outdated, especially without a Furthermore, it could be useful to split this into multiple PRs that can be reviewed and merged separately. This is not strictly needed, it could be done in separate commits. But smaller PRs are easier to create and review and get merged quickly. Once the first is merged, you can work on the next PR that builds on it. A separation could be:
I think the first two are probably simple enough to handle quickly. The last one can be trickier, and would require more manual playing out of scenarios, to see if things can break in unexpected corner cases. If that PR is small, it helps. Separately, and perhaps first, the existing code can be slightly cleaned up with changes like this: I would be happy to do that last myself. But if that interferes with what you want to do, then maybe not. Also one other security hint. One of the commits added some methods with a docstring. That would make such a method accessible through the web. In this case something like |

No description provided.