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

Work around Products.CMFCore and Products.CMFPlone design bug #1205

Merged
merged 1 commit into from
Apr 23, 2024

Conversation

d-maurer
Copy link
Contributor

Work around for #1202.

Products.CMFCore and Products.CMFPlone register non callable constructors with App.ProductContext.ProductContext.registerClass (even though the method's docstring clearly states that constructors must be callable).
Formerly, this was not a problem. But with the introduction of explicit zpublishable decoration, constructors are now required to be callable braking Products.CMFCore and Products.CMFPlone.

The PR introduces the new parameter resources of registerClass which should be used instead of constructors to register non callable objects. It uses work around code to avoid braking Products.CMFCore and Products.CMFPlone (and maybe other packages) when they abuse the constructors parameter for non callable objects; a DeprecationWarning is issued in this case.

In addition, the PR cleans up registerClass's docstring a bit: in the old days, when the product registry could still be persistent (controlled via a parameter), the constructors were required to be picklable. The feature was abandoned long ago (because of many problems) and consequently, constructors need no longer be picklable (indeed, the constructors generated for zpublishable decoration are not picklable). The PR removes the corresponding requirement from the docstring..

Finally, the PR removes the mention from the docstring that the instance_class parameter were not used (which is obviously wrong). But, I am not sure whether this removal is optimal. It is possible that the author wanted to express that instance_class is not used directly (to construct instances) but only to provide useful defaults for other parameters. Feedback welcome.

I would like to stress that this is a work around for bugs in other packages. They should get fixed there and the work around code here eventually removed.

Copy link
Contributor

@drfho drfho left a comment

Choose a reason for hiding this comment

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

works fine / LGTM / many thanks

Copy link
Member

@mauritsvanrees mauritsvanrees left a comment

Choose a reason for hiding this comment

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

I have not looked at the implementation details, but I confirm this fix works.
And I agree this should be fixed in CMFCore and CMFPlone soon.

@d-maurer d-maurer merged commit 22adba1 into master Apr 23, 2024
25 checks passed
@d-maurer d-maurer deleted the workaround_CMFCore_design_bug branch April 23, 2024 17:56
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.

None yet

4 participants