[fix] Preserve ZCA site for transaction hooks that depend on it #63
Conversation
The `zope.component` site should not be cleared before committing transacions in case any transaction hooks depend on it. In the test in which I ran into this issue, the `transaction.commit()` failed because the indexing queue was processing a `plone.app.blocks.indexing.LayoutSearchableText` indexer at commit-time that was trying to lookup a `plone.app.blocks.layoutbehavior.ILayoutAware` adapter. Since that adapter depends on behavior-derived interfaces, `plone.dexterity.schema` looks up a `plone.dexterity.interfaces.IDexterityFTI` utility for the `portal_type` which fails if the site isn't set which in turn yields no behavior interfaces which in turn makes the behavior-based adapter lookup fail. In my case, this wasn't even the symptom observed. Because the commit failed silently, the changes from applying the GS profile weren't persisted including, in my case, a custom theme browser layer registration which in turn meant a custom browser view lookup failed. IOW, this silent commit failure can lead to any number of very surprising, very wrong, and hard to diagnose behaviors. I'm not sure what changed or when, but this test worked under Python 2.7 and Plone 5.1 and I'm fairly confident that nothing changed in the testing code on "my" side that caused this and that the root change in behavior is somehow on the Plone side of things.
|
@rpatterson thanks for creating this Pull Request and help improve Plone! To ensure that these changes do not break other parts of Plone, the Plone test suite matrix needs to pass. Whenever you feel that the pull request is ready to be tested, either start all jenkins jobs pull requests by yourself, or simply add a comment in this pull request stating: With this simple comment all the jobs will be started automatically. Happy hacking! |
|
@jenkins-plone-org please run jobs |
|
I can't understand how the CI failures are related to my changes and I don't have the time to dig deeper. Hopefully this can get merged but if not at least it's here for others having the same problem to reference. |
|
@rpatterson Jenkins aborted the jobs after three hours. Our Jenkins instance is super slow right now, so this has nothing to do with your PR as far as I can see. If you have time, try to re-run the Jenkins job when there is low traffic on Jenkins. Sorry for the inconvenience! |
|
@jenkins-plone-org please run jobs |
1 similar comment
|
@jenkins-plone-org please run jobs |
|
Is this still a thing? |
The
zope.componentsite should not be cleared before committing transacionsin case any transaction hooks depend on it.
In the test in which I ran into this issue, the
transaction.commit()failedbecause the indexing queue was processing a
plone.app.blocks.indexing.LayoutSearchableTextindexer at commit-time thatwas trying to lookup a
plone.app.blocks.layoutbehavior.ILayoutAwareadapter.Since that adapter depends on behavior-derived interfaces,
plone.dexterity.schemalooks up aplone.dexterity.interfaces.IDexterityFTIutility for the
portal_typewhich fails if the site isn't set which in turnyields no behavior interfaces which in turn makes the behavior-based adapter
lookup fail.
In my case, this wasn't even the symptom observed. Because the commit failed
silently, the changes from applying the GS profile weren't persisted
including, in my case, a custom theme browser layer registration which in turn
meant a custom browser view lookup failed. IOW, this silent commit failure
can lead to any number of very surprising, very wrong, and hard to diagnose
behaviors.
I'm not sure what changed or when, but this test worked under Python 2.7 and
Plone 5.1 and I'm fairly confident that nothing changed in the testing code on
"my" side that caused this and that the root change in behavior is somehow on
the Plone side of things.