Hi Martijn, Shane I fixed some issues in zope.publisher and at the same time I implemented the default skin pattern within an adapter pattern. The adapter getDefaultSkin in zope.publisher.browser.py is registered in configure.zcml The changes are compatible within the zope core but only if zmcl is used. The concept is the following ---------------------------- A request implementation can provide a named IDefaultSkin adapter within the name "default". This default skin get used if no other default skin get defined. The IDefaultBrowserLayer is used as such a default adapter for IBrowserRequest. This was done hardcoded in setDefaultSkin before my changes. A default skin in your own projects get registered as an unnamed IDefaultSkin adapter. This is normaly done within the defaultSkin ZCML directive. Such a unnamed adapter get used before the setDefaultSkin method will lookup for the *fallback* named adapter. Everything is 100% compatible if the adapter configuration get used in configure.zcml. Questions --------- Everbody, Right now the setDefaultSkin method will silently fail if a skin doesn't provide ISkinType. What do you think should we raise an exception instead of silently ignore the skin? A general problem in the implemenation before and after my changes is that the skin interfaces registered by the defaultSkin ZCMl directive will register the interface as adapter which is not adaptable because it's not an adapter factory. Should we change the defaultSkin directive and register an adapter factory? This whould ensure that we don't have junk in the dapter registry. Another question The applySkin method removes all skin types from the request. Why do we not remove all skin types in setDefaultLayer? Are there any skins applied at the time we call setDefaultSkin? Martijn, Does grok need to register this new adapter somewhere? If the adapter configuration is missing the default skin apply pattern will break. Shane, Can you review and merge this changes into your zope.pipeline branch? Regards Roger Ineichen _____________________________ END OF MESSAGE
Roger Ineichen wrote:
Shane, Can you review and merge this changes into your zope.pipeline branch?
I'm going to put zope.pipeline on hold until the PyCon sprints. Jim and I need to discuss it in person; hopefully then I can understand his opposition and the group can decide on the best direction for zope.publisher and zope.app.publication. Shane
Shane Hathaway wrote:
Roger Ineichen wrote:
Shane, Can you review and merge this changes into your zope.pipeline branch?
I'm going to put zope.pipeline on hold until the PyCon sprints. Jim and I need to discuss it in person; hopefully then I can understand his opposition and the group can decide on the best direction for zope.publisher and zope.app.publication.
You should certainly talk to Jim if you think it will help you. But do note that Jim cannot be blocking your work anymore with a veto. In order to get this to be part of the Zope Framework, you'd need to convince the Zope Framework Steering Group, not Jim. I do think it's extremely bad if Jim's opposition can block all of your work on this project, which you were quite enthusiastic about before, for a whole month! This is exactly the sort of situation we want to avoid under our new system. As a Zope Framework Steering Group member I am quite supportive of your work, as reading your code already I have a much better sense of what's going on than I ever did with the standard publishing stack, so you got 1 out of 3 on board already. Regards, Martijn
Roger Ineichen wrote:
Does grok need to register this new adapter somewhere? If the adapter configuration is missing the default skin apply pattern will break.
As long as zope.publisher's configure.zcml does it, Grok will load that up. Grok isn't different in that respect; it only uses Grok-style registration in packages that explicitly use grok or grokcore.* libraries. It's quite possible Grok can start using some of your changes for its REST skin implementation (which do apply to IBrowserRequest), though I'm not 100% sure. I'm trying to understand why the default default skin cannot be registered as an interface, instead of the introduction of an adapter. Is this because defaultSkin can only be used for IBrowserRequest? I think you need to update the upgrade notes in zope3docs too to point out this change. If there is anything people should change to their code, you need to explain how to find what needs to be changed and what change to make. You need to at least warn them that something big changed if they get problems with their skins and point them to zope.publisher's changelog. I'll also note that this is *not* a minor bugfix release of zope.publisher, so it should be 3.6 not 3.5.7. Also don't use 'dev' markers in CHANGES.txt; only in setup.py. Regards, Martijn
Hi Martijn
Betreff: Re: [Zope-dev] zope.publisher
Roger Ineichen wrote:
Does grok need to register this new adapter somewhere? If the adapter configuration is missing the default skin apply pattern will break.
As long as zope.publisher's configure.zcml does it, Grok will load that up. Grok isn't different in that respect; it only uses Grok-style registration in packages that explicitly use grok or grokcore.* libraries.
It's quite possible Grok can start using some of your changes for its REST skin implementation (which do apply to IBrowserRequest), though I'm not 100% sure.
of corse, all changes are 100% compatible. The changes allow you to inherit from IHTTPRequest instead of IBrowserRequest and support a IDefaultSkin. The previous implementation required that the default skin pattern uses an IBrowserRequest. REST does normaly depend on IBrowserRequest but JSON-RPC doesn't have to, or you will get all the IBrwoserRequest parts in JSON-RPC too which is not good. Also XML-RPC has to be inherited from IHTTPRequest and not how it is right now from IBrowserRequest. XML-RPC should also allow to use it's own default skin and not depend on the IBrowserRequest default skin implementation. e.g. we do really not need a contents.html page in XML-RPC skins. But that's another (security) topic which we can discuss at a later time.
I'm trying to understand why the default default skin cannot be registered as an interface, instead of the introduction of an adapter. Is this because defaultSkin can only be used for IBrowserRequest?
Because it's not an adapter. It's doens't provide what it should provide and is registered for. The following will end in a TypeError:
request BrowserRequest() defautlSkin = IDefaultskin(request) TypeError ...
but it should return: IDefaultBrowserLayer Another side effect is that we get pickled interfaces in the adapter registry. That really hurts if someone does the same in a local (persistent) adapter registry and the interface get removed. The question is, should we allow to register such *junk* in the zope adapter registry?
I think you need to update the upgrade notes in zope3docs too to point out this change. If there is anything people should change to their code, you need to explain how to find what needs to be changed and what change to make. You need to at least warn them that something big changed if they get problems with their skins and point them to zope.publisher's changelog.
will do so soon
I'll also note that this is *not* a minor bugfix release of zope.publisher, so it should be 3.6 not 3.5.7. Also don't use 'dev' markers in CHANGES.txt; only in setup.py.
agreed Regards Roger Ineichen
Regards,
Martijn
_______________________________________________ Zope-Dev maillist - Zope-Dev@zope.org http://mail.zope.org/mailman/listinfo/zope-dev ** No cross posts or HTML encoding! ** (Related lists - http://mail.zope.org/mailman/listinfo/zope-announce http://mail.zope.org/mailman/listinfo/zope )
participants (3)
-
Martijn Faassen -
Roger Ineichen -
Shane Hathaway