package dependency refactoring progress report
Hi there, This is a progress report on the package dependency refactoring work. We've had a lot of people contribute to this process (thanks everybody!), and bit by bit we are able to make a serious impact on dependencies. Yay! Let's take for example zope.app.publisher, which a few weeks ago had a dependency structure like this: http://startifact.com/depgraphs/zope.app.publisher-before.svg (60 dependencies, including zope.app.form and zope.formlib). After a lot of work on a lot of packages, we now have a dependency structure like this: http://startifact.com/depgraphs/zope.app.publisher-after.svg (45 dependencies, no more form related stuff) Still a big graph, but a lot simpler nonetheless, and down 15 packages. So, due to the recent changes we've now managed to cut away zope.app.form and zope.formlib entirely from zope.app.publisher's dependency chain (except for the tests). The main dependency cycles we started out with in the big graph were these: http://startifact.com/depgraphs/zope_app_publisher_cycles.svg After some work we'd gotten it down to this: http://startifact.com/depgraphs/zope_app_publisher_cycles2.svg And by now the main cycles left are these: http://startifact.com/depgraphs/zope_app_publisher_cycles3.svg So, the only dependency cycles left in zope.app.publisher are these: zope.app.publisher <--> zope.app.publication <--> zope.app.http And also this (familiar) one: zope.container <--> zope.filerepresentation I need to do some more analysis to see what other complexities we have in the dependency chain in the ZTK. One obvious set of tasks is to continue evaluating dependencies of things like zope.app.publisher to see whether we cannot take out a few more. Just study the graph and see what can be done by examining the code: http://startifact.com/depgraphs/zope.app.publisher-after.svg For instance, what is zope.app.pagetemplate doing depending on zope.dublincore? What about its direct dependency on zope.container? Another set of tasks is to examine all the test dependencies we have, as those are way more complicated than the main dependencies. Ideally our test dependencies shrink to nothing as well. This will be a hard slog as we rewrite tests, but we can target one dependency at the time, too. Regards, Martijn
On 5/22/09 1:11 PM, Martijn Faassen wrote:
After some work we'd gotten it down to this:
http://startifact.com/depgraphs/zope_app_publisher_cycles2.svg
And by now the main cycles left are these:
http://startifact.com/depgraphs/zope_app_publisher_cycles3.svg
So, the only dependency cycles left in zope.app.publisher are these:
zope.app.publisher<--> zope.app.publication<--> zope.app.http
In isolation, it sort of looks like we should just collapse zope.app.publication and zope.app.http *into* zope.app.publisher, given that the relationships between the packages are all cyclical. - C
Chris McDonough wrote:
On 5/22/09 1:11 PM, Martijn Faassen wrote:
After some work we'd gotten it down to this:
http://startifact.com/depgraphs/zope_app_publisher_cycles2.svg
And by now the main cycles left are these:
http://startifact.com/depgraphs/zope_app_publisher_cycles3.svg
So, the only dependency cycles left in zope.app.publisher are these:
zope.app.publisher<--> zope.app.publication<--> zope.app.http
In isolation, it sort of looks like we should just collapse zope.app.publication and zope.app.http *into* zope.app.publisher, given that the relationships between the packages are all cyclical.
Well, zope.app.publication and zope.app.publisher are really completely different, despite their similar names. zope.app.publication is a publisher implementation, while zope.app.publisher contains publishable resources. IMHO, zope.app.publisher is more broadly useful than zope.app.publication. So I'm working on removing the dependency from zope.app.publisher to zope.app.publication. It's mostly done, but I'm writing tests for the code I'm moving, since it never had any tests... I'm thinking about also removing the reverse dependency (zope.app.publication -> zope.app.publisher). I think the default view name logic might belong in publication, not publisher. Shane
Martijn Faassen wrote:
So, the only dependency cycles left in zope.app.publisher are these:
zope.app.publisher <--> zope.app.publication <--> zope.app.http
I fixed those tonight. On the trunk, there are no longer any dependencies between zope.app.publisher, zope.app.publication, and zope.app.http, except testing dependencies. Here is a summary of what I did: - Moved the publication traversal code to zope.traversing and simplified it from 3 classes to 1. This did not increase the dependencies of zope.traversing at all, since publication traversal is only slightly different from the traversal logic already in zope.traversing. It also opens the doors for someday moving the gnarly traversal code in zope.publisher to something saner in zope.traversing. - Moved an XML-RPC view registration from zope.app.publisher to zope.app.publication. - Moved IHTTPException, IMethodNotAllowed, and the MethodNotAllowed exception class to zope.publisher.interfaces.http. This move decoupled zope.app.http from zope.app.publication. - Moved getDefaultViewName() and queryDefaultViewName() to zope.publisher.defaultview. If you look at the commit messages, you'll see that I first attempted a different move that included the <browser:defaultView> handler, but that move turned out wrong for other packages and I reverted it. - I used zope.deferred to deprecate things I moved from zope.app.publication, zope.app.publisher, and zope.app.http. Are there any objections to using zope.deferred in those packages? In all, I changed 6 packages. Should I release them now, or should I look for other dependencies we might clean up at the same time? The changed packages are: zope.traversing (3.7.0) zope.publisher (3.8.0) zope.app.publisher (3.8.0) zope.app.publication (3.7.0) zope.app.http (3.6.0) zope.app.zcmlfiles (3.5.5; bugfix only) Shane
Shane Hathaway wrote:
Martijn Faassen wrote:
So, the only dependency cycles left in zope.app.publisher are these:
zope.app.publisher <--> zope.app.publication <--> zope.app.http
I fixed those tonight. On the trunk, there are no longer any dependencies between zope.app.publisher, zope.app.publication, and zope.app.http, except testing dependencies.
I should take a moment to describe the different purposes of these packages as I see them now. Conceptually, they are really quite independent. - zope.app.publisher: A library of ZCML directives for configuring views. Also provides generic view classes. A better name for this package might be "zope.basicviews". A lot of packages depend on this. - zope.app.publication: Provides IPublication implementations and a mechanism/registry for choosing a different publication class for each request. Most packages should not depend on this. A better name might be "zope.publicationregistry". - zope.app.http: Provides generic views that translate HTTP verbs like PUT, DELETE, and OPTIONS into map operations. The idea is clever, but not everyone needs a REST-style API. A better name might be "zope.httpverbs". Shane
Hey Shane, Shane Hathaway wrote:
Shane Hathaway wrote:
Martijn Faassen wrote:
So, the only dependency cycles left in zope.app.publisher are these:
zope.app.publisher <--> zope.app.publication <--> zope.app.http I fixed those tonight. On the trunk, there are no longer any dependencies between zope.app.publisher, zope.app.publication, and zope.app.http, except testing dependencies.
I should take a moment to describe the different purposes of these packages as I see them now. Conceptually, they are really quite independent.
Thanks for doing this analysis, and considering improved naming. I think good naming is very important, and it will help get this functionality out of the 'zope.app' ghetto.
- zope.app.publisher: A library of ZCML directives for configuring views. Also provides generic view classes. A better name for this package might be "zope.basicviews". A lot of packages depend on this.
I'm not sure 'basic' needs to be in there. Do we have anything less basic? What about simply calling it zope.view? (I don't like the plural in package names either generally)
- zope.app.publication: Provides IPublication implementations and a mechanism/registry for choosing a different publication class for each request. Most packages should not depend on this. A better name might be "zope.publicationregistry".
I'm fine with this. I was considering 'zope.publication', but we already have 'zope.publisher' so that'd get very confusing again, something we should avoid.
- zope.app.http: Provides generic views that translate HTTP verbs like PUT, DELETE, and OPTIONS into map operations. The idea is clever, but not everyone needs a REST-style API. A better name might be "zope.httpverbs".
Even though I don't really like the plural, I think 'zope.http' would promise a bit too much, so 'zope.httpverbs' sound better. So if we get some consensus about this, we need volunteers that can help move the code over to these new packages and leave backwards compatible imports in the old places. Is there anything in these packages we can safely leave behind do you think? (ZMI related, perhaps?) Regards, Martijn
On Sat, May 23, 2009 at 08:59:34PM +0200, Martijn Faassen wrote:
What about simply calling it zope.view? (I don't like the plural in package names either generally)
FWIW at some point I decided that singular is appropriate when a package defines a concept, and plural is appropriate when a package provides multiple implementations of that concept. Thus you have zope.interface but zope.$anything.interfaces. Marius Gedminas -- http://pov.lt/ -- Zope 3 consulting and development
Marius Gedminas wrote:
On Sat, May 23, 2009 at 08:59:34PM +0200, Martijn Faassen wrote:
What about simply calling it zope.view? (I don't like the plural in package names either generally)
FWIW at some point I decided that singular is appropriate when a package defines a concept, and plural is appropriate when a package provides multiple implementations of that concept.
Thus you have zope.interface but zope.$anything.interfaces.
In this case it's a plural of metaconfiguration. Maybe "zope.viewzcml"? I think that would express the intent even better. Shane
Martijn Faassen wrote:
Shane Hathaway wrote:
- zope.app.publisher: A library of ZCML directives for configuring views. Also provides generic view classes. A better name for this package might be "zope.basicviews". A lot of packages depend on this.
I'm not sure 'basic' needs to be in there. Do we have anything less basic?
What about simply calling it zope.view? (I don't like the plural in package names either generally)
Sounds good to me.
- zope.app.publication: Provides IPublication implementations and a mechanism/registry for choosing a different publication class for each request. Most packages should not depend on this. A better name might be "zope.publicationregistry".
I'm fine with this. I was considering 'zope.publication', but we already have 'zope.publisher' so that'd get very confusing again, something we should avoid.
Right.
- zope.app.http: Provides generic views that translate HTTP verbs like PUT, DELETE, and OPTIONS into map operations. The idea is clever, but not everyone needs a REST-style API. A better name might be "zope.httpverbs".
Even though I don't really like the plural, I think 'zope.http' would promise a bit too much, so 'zope.httpverbs' sound better.
Another option is "zope.rest", because a simple REST interface is what the package tries to accomplish.
So if we get some consensus about this, we need volunteers that can help move the code over to these new packages and leave backwards compatible imports in the old places. Is there anything in these packages we can safely leave behind do you think? (ZMI related, perhaps?)
I haven't come across anything we'd want to leave behind. Summarizing: zope.app.publisher -> zope.view zope.app.publication -> zope.publicationregistry zope.app.http -> zope.rest Shane
Hi there, Shane Hathaway wrote: [snip]
Summarizing:
zope.app.publisher -> zope.view zope.app.publication -> zope.publicationregistry zope.app.http -> zope.rest
I'm not sure about zope.rest; there's already a z3c.rest which likely does something quite different, and I think a "zope.rest" package should actually *talk* about REST. What about "zope.httpview" instead? Another note, I think we should consider splitting off zope.app.publisher.xmlrpc, which looks quite independent from the rest, into its own package. So: zope.app.publisher -> zope.view, zope.xmlrpcview One question is whether zope.httpview and zope.xmlrpcview are really similar enough to warrant such a similar naming convention. zope.app.publisher.xmlrpc does not only define some XMLRPC-related views, it also defines a ZCML directive, which zope.httpview doesn't do. I also wonder what should happen with zope.publisher.xmlrpc (which also looks quite independent from the rest of the zope.publisher code). Regards, Martijn
Martijn Faassen wrote:
I'm not sure about zope.rest; there's already a z3c.rest which likely does something quite different, and I think a "zope.rest" package should actually *talk* about REST. What about "zope.httpview" instead?
Well, no, I don't think it's generic enough to call that. zope.app.http is almost a webdav implementation, except I'm not sure it implements enough to actually work with most webdav clients.
Another note, I think we should consider splitting off zope.app.publisher.xmlrpc, which looks quite independent from the rest, into its own package. So:
zope.app.publisher -> zope.view, zope.xmlrpcview
I've pondered this split before, but AFAICT it would only increase the number of dependencies, so I've been waiting for the graph to shrink before talking about it. Shane
Shane Hathaway wrote:
Martijn Faassen wrote:
I'm not sure about zope.rest; there's already a z3c.rest which likely does something quite different, and I think a "zope.rest" package should actually *talk* about REST. What about "zope.httpview" instead?
Well, no, I don't think it's generic enough to call that. zope.app.http is almost a webdav implementation, except I'm not sure it implements enough to actually work with most webdav clients.
Maybe we'll leave this behind in zope.app.* space for the moment and focus on the others, then?
Another note, I think we should consider splitting off zope.app.publisher.xmlrpc, which looks quite independent from the rest, into its own package. So:
zope.app.publisher -> zope.view, zope.xmlrpcview
I've pondered this split before, but AFAICT it would only increase the number of dependencies, so I've been waiting for the graph to shrink before talking about it.
It would allow a whole chunk of code to be cut out for those people who don't care about XMLRPC or don't even want to enable it on their server. The reason I bring it up now is because this split would be best done while we are moving things out of zope.app.publisher anyway. If we did it afterwards, we'd need a backwards compatibility dependency from zope.view on zope.xmlrpcview. Regards, Martijn
Shane Hathaway wrote:
Martijn Faassen wrote:
So, the only dependency cycles left in zope.app.publisher are these:
zope.app.publisher <--> zope.app.publication <--> zope.app.http
I fixed those tonight. On the trunk, there are no longer any dependencies between zope.app.publisher, zope.app.publication, and zope.app.http, except testing dependencies.
Cool! Thanks very much!
Here is a summary of what I did: [snip summary] - I used zope.deferred to deprecate things I moved from zope.app.publication, zope.app.publisher, and zope.app.http. Are there any objections to using zope.deferred in those packages?
No objection, but what's the reason to use zope.deferred instead of straight imports or zope.deprecation? To break the dependencies?
In all, I changed 6 packages. Should I release them now, or should I look for other dependencies we might clean up at the same time?
I'm +1 on releasing now. (and thanks for making them feature releases) Getting these things out there gives everybody who plays with this nicer dependency graphs and this tends to help improvement. Regards, Martijn
Martijn Faassen wrote:
Shane Hathaway wrote:
- I used zope.deferred to deprecate things I moved from zope.app.publication, zope.app.publisher, and zope.app.http. Are there any objections to using zope.deferred in those packages?
No objection, but what's the reason to use zope.deferred instead of straight imports or zope.deprecation? To break the dependencies?
I had forgotten about zope.deprecation. :-) I'll switch to that.
In all, I changed 6 packages. Should I release them now, or should I look for other dependencies we might clean up at the same time?
I'm +1 on releasing now. (and thanks for making them feature releases) Getting these things out there gives everybody who plays with this nicer dependency graphs and this tends to help improvement.
Ok, I intend to release them today. Shane
Shane Hathaway wrote:
Martijn Faassen wrote:
Shane Hathaway wrote:
In all, I changed 6 packages. Should I release them now, or should I look for other dependencies we might clean up at the same time? I'm +1 on releasing now. (and thanks for making them feature releases) Getting these things out there gives everybody who plays with this nicer dependency graphs and this tends to help improvement.
Ok, I intend to release them today.
Done. I look forward to seeing the changes in the dependency graph. Shane
Hey, Shane Hathaway wrote:
Done. I look forward to seeing the changes in the dependency graph.
Great, thanks! The only cycle left in the zope.app.publisher graph is between zope.container and zope.filerepresentation. The graph now contains 42 notes, so we got rid of 3 more dependencies. Here it is: http://startifact.com/depgraphs/zope.app.publisher-after2.svg It's interesting to see zope.deprecation is a new dependency. We could consider changing these deprecations to simple imports if this is possible... Knowing there are no cycles besides the zope.container one, this graph starts to look comprehensible, that's good. :) Here's zope.app.publication (same zope.container cycle, no other cycles); http://startifact.com/depgraphs/zope.app.publication.svg And here's zope.app.http: http://startifact.com/depgraphs/zope.app.http.svg (again the zope.container cycle, nothing else) Regards, Martijn
Martijn Faassen wrote:
It's interesting to see zope.deprecation is a new dependency. We could consider changing these deprecations to simple imports if this is possible...
Certainly, but what is the right way to deprecate code, then? I'm not very happy about including zope.deprecation either, since it uses the sys.modules trick just like zope.deferredimport, but those deprecation warnings are important, aren't they?
Knowing there are no cycles besides the zope.container one, this graph starts to look comprehensible, that's good. :)
It's still really big though. Hmph. Thanks for the new graphs! Shane
Shane Hathaway wrote:
Martijn Faassen wrote:
It's interesting to see zope.deprecation is a new dependency. We could consider changing these deprecations to simple imports if this is possible...
Certainly, but what is the right way to deprecate code, then?
We've just started to do imports instead, with a BBB comment. The idea is that tools exist (or almost-exist) that can report on indirect imports; the test runner has such an extension, though I believe it's still sitting on a branch. The idea is also that plain imports are better understood by random Python programmers.
Knowing there are no cycles besides the zope.container one, this graph starts to look comprehensible, that's good. :)
It's still really big though. Hmph.
Yes. I think zope.container and zope.site are interesting candidates to look at removing as dependencies. I saw one dependency on getSite in zope.app.publisher (the rest are test dependencies)... I wish we could separate zope.site into two packages somehow. One would just contain the interfaces describing how to get to a site, and a way to easily *test* with sites, a testing module (I have some code sitting around that could help there). The other would actually implement a site in relation to containers. This work might be a good opportunity as well to think about renaming the word "site" to something else, as "site" isn't that great a word for a local component registry. The only direct dependency on zope.container (not through zope.site) is in zope.app.publisher.xmlrpc, in ZCML (see other discussion about zope.xmlrpcview, another reason to split this off :). The dependency of zope.app.pagetemplate on zope.dublincore is also relatively weak. Regards, Martijn
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Shane Hathaway wrote:
Martijn Faassen wrote:
It's interesting to see zope.deprecation is a new dependency. We could consider changing these deprecations to simple imports if this is possible...
Certainly, but what is the right way to deprecate code, then? I'm not very happy about including zope.deprecation either, since it uses the sys.modules trick just like zope.deferredimport, but those deprecation warnings are important, aren't they?
Not so much, IMO. I think we can leave simple BBB imports in place, warning-free, forever (with a '#BBB' comment so that people don't delete them as unused imports). Tres. - -- =================================================================== Tres Seaver +1 540-429-0999 tseaver@palladion.com Palladion Software "Excellence by Design" http://palladion.com -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.6 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org iD8DBQFKGqrL+gerLs4ltQ4RAmntAJoC0m6i4JKNLStPhFjxSzafVwi5WQCePYIt aFTuMo+AEkc/j06AmEvZVRY= =PgZh -----END PGP SIGNATURE-----
Tres Seaver wrote:
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
Shane Hathaway wrote:
Martijn Faassen wrote:
It's interesting to see zope.deprecation is a new dependency. We could consider changing these deprecations to simple imports if this is possible... Certainly, but what is the right way to deprecate code, then? I'm not very happy about including zope.deprecation either, since it uses the sys.modules trick just like zope.deferredimport, but those deprecation warnings are important, aren't they?
Not so much, IMO. I think we can leave simple BBB imports in place, warning-free, forever (with a '#BBB' comment so that people don't delete them as unused imports).
Ok, I've replaced all the zope.deprecation imports I added with BBB. What does BBB stand for anyway? Shane
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Shane Hathaway wrote:
Ok, I've replaced all the zope.deprecation imports I added with BBB. What does BBB stand for anyway?
"Backward compatibility": we started using it as an easy-to-find marker in code some time back. Tres. - -- =================================================================== Tres Seaver +1 540-429-0999 tseaver@palladion.com Palladion Software "Excellence by Design" http://palladion.com -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.6 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org iD8DBQFKGs6q+gerLs4ltQ4RAnpmAJ47Mp0Sq2jTMHhMIHN7CZUciqlSEQCffmzd OojorvtM5cvb9++J7ce6/ls= =g20Z -----END PGP SIGNATURE-----
participants (5)
-
Chris McDonough -
Marius Gedminas -
Martijn Faassen -
Shane Hathaway -
Tres Seaver