Re: [Framework-Team] PLIP #247 ready for review (I think)
On Sun, Jan 18, 2009 at 8:48 PM, Israel Saeta Pérez dukeb...@gmail.comwrote: On Sat, Jan 17, 2009 at 2:37 AM, Ethan Jucovy ethan.juc...@gmail.comwrote: Some of the things that people have said ought to be discussed about PLIP# 247 are: 1) impact on server startup time 2) implications for test environments 3) stabilizing and releasing a new version of z3c.autoinclude 4) debugging tools/APIs for z3c.autoinclude * 5) documentation and/or code generation templates to explain the entry point for plugin authors* I'll begin working on #3 immediately, since there's plenty I can do there while discussion is still happening. Please let me know if there's anything else I'll need to submit for the PLIP's review. I'm not a member of the Framework Team so this is not mandatory for the PLIP's review, but as a member of the Documentation Team I would appreciate number (5). Something like Before, we did this to accomplish this task. Now, we can do that., as Wichert (?) suggested, so we would be able to find and update affected docs easily. Thanks in advance! The buildout manual has been updated (but updates are still private) to reflect this new feature: http://plone.org/documentation/tutorial/buildout/copy_of_creating-a-new-package See the new Automate ZCML loading for your package section. I would appreciate any review input since I'm not sure I've got it right. Thanks! -- israel Documentation Team ___ Framework-Team mailing list Framework-Team@lists.plone.org http://lists.plone.org/mailman/listinfo/framework-team
Re: [Framework-Team] PLIP #247 ready for review (I think)
On Feb 7, 2009, at 12:56 AM, Ethan Jucovy wrote:
On Thu, Feb 5, 2009 at 3:37 PM, Andreas Zeidler a...@zitc.de wrote:
On Feb 5, 2009, at 9:29 PM, Andreas Zeidler wrote:
i noticed that. in fact, i was trying to figure out how the
assigned value (plone) ended up in `module_name`. i
ended up looked at the entry point's (the one you get from
`pkg_resources.iter_entry_points`) `__dict__`, assuming
that some magic in `pkg_resources` would translate the target to
be meant as the module's name. but apparently
all given value end up as `module_name` in the dict. i suppose i
should have had a closer look at entry point definitions first.
Ah, yeah, a single entry point basically models a dictionary entry:
`module_name` is (part of) the entry point's value, which is
imported as a Python object; `name` is the key.
i was missing that info initially, but figured as much after a bit...
thanks for clarifying it again, though.
Perhaps z3c.autoinclude could check if
`os.environ.has_key('Z3C_AUTOINCLUDE_DISABLED')` and the test
runner and/or a buildout option could set that environment
variable?
yep, also +1. after all, that's exactly what environment variable
are for, aren't they? :)
OK, z3c.autoinclude trunk as of r96193 checks for
'Z3C_AUTOINCLUDE_PLUGINS_DISABLED' and
'Z3C_AUTOINCLUDE_DEPENDENCIES_DISABLED' in the environment.
great. just one more tiny little suggestion: how about catching the
key error in the `enable_*` function, so people can use it
unconditionally (i.e. without consulting `dependencies_disabled` first)?
$ ./bin/zopepy
from z3c.autoinclude.api import enable_dependencies
enable_dependencies()
Traceback (most recent call last):
[...]
KeyError: 'Z3C_AUTOINCLUDE_PLUGINS_DISABLED'
n/p, none of this has been too time-consuming yet :)
:)
thanks for the update, ethan. i'm changing my vote to +1.
cheers,
andi
--
zeidler it consulting - http://zitc.de/ - i...@zitc.de
friedelstraße 31 - 12047 berlin - telefon +49 30 25563779
pgp key at http://zitc.de/pgp - http://wwwkeys.de.pgp.net/
plone 3.2.1 released! -- http://plone.org/products/plone/
PGP.sig
Description: This is a digitally signed message part
___
Framework-Team mailing list
Framework-Team@lists.plone.org
http://lists.plone.org/mailman/listinfo/framework-team
Re: [Framework-Team] PLIP #247 ready for review (I think)
On Jan 17, 2009, at 2:37 AM, Ethan Jucovy wrote: Hello, hi ethan, I've implemented PLIP #247, Automate ZCML Loading for Plone Plugins (http://plone.org/products/plone/roadmap/247 ) and committed a review buildout here: https://svn.plone.org/svn/plone/review/plip247-automate-plugin-zcml for your review. i've added my review notes in http://dev.plone.org/plone/changeset/ 24863 (and r24864). for now i'm +0, but this can be fixed quite easily... :) cheers, andi -- zeidler it consulting - http://zitc.de/ - i...@zitc.de friedelstraße 31 - 12047 berlin - telefon +49 30 25563779 pgp key at http://zitc.de/pgp - http://wwwkeys.de.pgp.net/ plone 3.2rc1 released! -- http://plone.org/products/plone/ PGP.sig Description: This is a digitally signed message part ___ Framework-Team mailing list Framework-Team@lists.plone.org http://lists.plone.org/mailman/listinfo/framework-team
Re: [Framework-Team] PLIP #247 ready for review (I think)
On Feb 5, 2009, at 6:35 PM, Ethan Jucovy wrote:
Hey Andi,
hi ethan,
Thanks for the review.
thanks for the quick reply! :)
Again I'll have to be a bit brief, this time because I'm about to
get on a plane. :) But I figured I may as well give some quick
replies to keep the conversation going.
much appreciated. oh, and sorry for the late review, btw. i'm not
sure you saw my message yesterday, but unfortunately a few things kept
me from doing it earlier (lack of sleep amongst them :)).
Let me know if I should be putting these comments in SVN or on http://plone.org/products/plone/roadmap/247
instead of//as well as replying on-list, BTW.
i guess the list is fine, as interested parties can still read the
entire discussion in the archives, for example by following the link i
put on the PLIP page. you may of course also answer there or link to
your answer here. i don't think we have any strict policy in place
about this. :)
At http://dev.plone.org/plone/changeset/24863, Andreas Zeidler a...@zitc.de
wrote:
* one issue i was wondering about is how to target multiple
base packages
for the plugin case. adding an additional target = ... line
yields
a Duplicate entry point error in `easy_install`. reading the
code
target = plone, grok isn't supported either. seeing that
grok is
already actively using the package i would imagine that
supporting
multiple base packages (or projects) does make sense,
though. adding
such support would be a definite plus, imho, and should also be
rather
simple to do.
This is actually already a hidden feature. ;) The entry point name
(target) is purely convention; it's not used at all in the code.
i noticed that. in fact, i was trying to figure out how the assigned
value (plone) ended up in `module_name`. i ended up looked at the
entry point's (the one you get from `pkg_resources.iter_entry_points`)
`__dict__`, assuming that some magic in `pkg_resources` would
translate the target to be meant as the module's name. but
apparently all given value end up as `module_name` in the dict. i
suppose i should have had a closer look at entry point definitions
first.
So
{{{
entry_points=
[z3c.autoinclude.plugin]
morx = plone
bar = mygrok.app
}}}
yes, that works. in any case this should be documented, though.
Big +1 on considering the testing complications. (Though in my
limited experience I've actually found that any of my tests that
break as a result of z3c.autoinclude's unpredictable ZCML
inclusion are badly isolated tests anyway.)
well, i was thinking of a debug session where some tests where failing
after LinguaPlone had been added to the buildout. i'm not entirely
sure anymore if i had already added it to the buildout (zcml-wise) or
if the module was loaded by the testrunner thereby activating some
monkey-patches.
but you're right, that might also qualify as bad test isolation, but
every little bit that helps making this easier is much appreciated
(and i know not just by me :)).
My gut feeling is that I'd rather keep logic like what to do in test
runs, or for that matter what a test run is, out of z3c.autoinclude
though.
+1, of course. that wouldn't make sense, imho. there are too many
different testrunners for one, and there will probably be others in
the future. knowing them all is certainly not something
`z3c.autoinclude` should attempt.
Perhaps z3c.autoinclude could check if
`os.environ.has_key('Z3C_AUTOINCLUDE_DISABLED')` and the test runner
and/or a buildout option could set that environment variable?
yep, also +1. after all, that's exactly what environment variable are
for, aren't they? :)
cheers,
andi
--
zeidler it consulting - http://zitc.de/ - i...@zitc.de
friedelstraße 31 - 12047 berlin - telefon +49 30 25563779
pgp key at http://zitc.de/pgp - http://wwwkeys.de.pgp.net/
plone 3.2rc1 released! -- http://plone.org/products/plone/
PGP.sig
Description: This is a digitally signed message part
___
Framework-Team mailing list
Framework-Team@lists.plone.org
http://lists.plone.org/mailman/listinfo/framework-team
Re: [Framework-Team] PLIP #247 ready for review (I think)
On Feb 5, 2009, at 9:29 PM, Andreas Zeidler wrote:
[...snip...]
yes, that works. in any case this should be documented, though.
[...snip again...]
Perhaps z3c.autoinclude could check if
`os.environ.has_key('Z3C_AUTOINCLUDE_DISABLED')` and the test
runner and/or a buildout option could set that environment variable?
yep, also +1. after all, that's exactly what environment variable
are for, aren't they? :)
i forgot to mention that it'd be great if you could try to work these
out until some time next week. technically the revision deadline is
this saturday, of course, but since my review was several days late, i
think it's only fair to still grant you a full week to address these
issues. i hope the delay doesn't cause too many inconveniences and
you'll find the time it takes...
cheers,
andi
--
zeidler it consulting - http://zitc.de/ - i...@zitc.de
friedelstraße 31 - 12047 berlin - telefon +49 30 25563779
pgp key at http://zitc.de/pgp - http://wwwkeys.de.pgp.net/
plone 3.2rc1 released! -- http://plone.org/products/plone/
PGP.sig
Description: This is a digitally signed message part
___
Framework-Team mailing list
Framework-Team@lists.plone.org
http://lists.plone.org/mailman/listinfo/framework-team
Re: [Framework-Team] PLIP #247 ready for review (I think)
I just spent three hours writing a detailed email explaining the situation, but gmail seems to have completely swallowed it, drafts and all, when I hit send. I'm travelling right now and have very limited internet connectivity here, so I'm just going to try to briefly summarize my original reply; if further clarification is needed, let me know and I'll provide more information as soon as I can. Sorry to be so terse, this gmail accident is just making me *really* grumpy. :) * the test failures are on the trunk of z3c.autoinclude; releases have been stable and are tested on multiple OSes and python environments before I cut them. i'll continue to be careful about this in the future. * the tests pass (for me at least) in the environment created by z3c.autoinclude's standalone buildout -- which is the only way I thought to run them before submitting the plip * after seeing your reply, I tried running the tests in the PLIP buildout's environment with `bin/instance test -s z3c.autoinclude` and did see failures * i've now fixed them * can you reconfirm that the failures you saw were from utils.txt? the ones i saw were from plugin.txt (though the assertion that was triggered was indeed the alarming one in utils.py) * briefly, the problem was that the foo module in the demonstration package i provided collided with another foo module that's installed in z3c.autoinclude's test environment (z3c.autoinclude/src/z3c/autoinclude/tests/FooPackage is the egg). moral: namespaces are indeed a good idea. :) i've renamed the demo package to plip247.foo and will also namespace z3c.autoinclude's test packages before the next release so this doesn't happen again * the reason that assertion was so alarming and vague is because, when i wrote the code, i had no idea what sort of situation could possibly trigger it. so i figured i might as well throw in an assertion, even if // because i couldn't imagine what its failure would mean. now that it's actually popped up i think i understand it: it's asserting that no more than one non-empty-virtual-namespace-module named foo (for any string foo) will be importable simultaneously. i'll make the assertion less alarming and more informative as soon as i figure out a concise way to say that. * do the tests pass for you now in a fresh buildout // if you svn up and re-install the foo demo package? again, they are passing for me now in this environment, let me know if you're seeing different failures. Thanks again for taking the time to look at this, I really appreciate all of your notes (and the buildout help!) I'll try to be as responsive as I possibly can. thanks, ethan On Sun, Jan 25, 2009 at 9:31 AM, Martijn Pieters m...@zopatista.com wrote: On Sat, Jan 17, 2009 at 02:37, Ethan Jucovy ethan.juc...@gmail.com wrote: I've implemented PLIP #247, Automate ZCML Loading for Plone Plugins (http://plone.org/products/plone/roadmap/247) and committed a review buildout here: https://svn.plone.org/svn/plone/review/plip247-automate-plugin-zcml for your review. I have started my review. To my dismay, however, the z3c.autoinclude package has several test failures. I understand that this package is also being used by the Grok project, and it appears to work as advertised, but test failures is not a good sign of package quality. For example, utils.txt somehow triggers an assert on line 99 in utils.py, which has a somewhat distressing comment: assert len(non_namespaced_dists) == 1 ### we really are in trouble if we get into a situation with more than one Could you comment on the state of these failures? The buildout is branched from the 3.3 review bundle buildout based on buildout.eggtractor, which automatically generates ZCML slugs for development packages and puts them in parts/instance/etc/package- includes. Since the purpose of PLIP 247 is to automate ZCML loading, those automatically generated ZCML slugs should be removed. I've hardly ever used buildout so I'm not sure how to change that behavior -- sorry about that; I'll change it if I can figure out how. Simply *not* use buildout.eggtractor; you only have 3 eggs in this buildout, add them to the develop, eggs and zcml lines and Bob's your uncle. Alternatively, just set tractor-autoload-zcml to false, which you did. -- Martijn Pieters ___ Framework-Team mailing list Framework-Team@lists.plone.org http://lists.plone.org/mailman/listinfo/framework-team
Re: [Framework-Team] PLIP #247 ready for review (I think)
On Fri, Jan 30, 2009 at 21:00, Ethan Jucovy ethan.juc...@gmail.com wrote: * after seeing your reply, I tried running the tests in the PLIP buildout's environment with `bin/instance test -s z3c.autoinclude` and did see failures * i've now fixed them Yay! :-) * can you reconfirm that the failures you saw were from utils.txt? the ones i saw were from plugin.txt (though the assertion that was triggered was indeed the alarming one in utils.py) * briefly, the problem was that the foo module in the demonstration package i provided collided with another foo module that's installed in z3c.autoinclude's test environment (z3c.autoinclude/src/z3c/autoinclude/tests/FooPackage is the egg). moral: namespaces are indeed a good idea. :) i've renamed the demo package to plip247.foo and will also namespace z3c.autoinclude's test packages before the next release so this doesn't happen again I've updated the foo.cfg buildout config to reflect this. * the reason that assertion was so alarming and vague is because, when i wrote the code, i had no idea what sort of situation could possibly trigger it. so i figured i might as well throw in an assertion, even if // because i couldn't imagine what its failure would mean. now that it's actually popped up i think i understand it: it's asserting that no more than one non-empty-virtual-namespace-module named foo (for any string foo) will be importable simultaneously. i'll make the assertion less alarming and more informative as soon as i figure out a concise way to say that. Great! It's a good idea to explicitly warn about this. * do the tests pass for you now in a fresh buildout // if you svn up and re-install the foo demo package? again, they are passing for me now in this environment, let me know if you're seeing different failures. The tests now all pass, I've upgraded my review verdict to a +1. :-) Thanks again for taking the time to look at this, I really appreciate all of your notes (and the buildout help!) I'll try to be as responsive as I possibly can. np! Thanks for your hard work on implementing the PLIP! -- Martijn Pieters ___ Framework-Team mailing list Framework-Team@lists.plone.org http://lists.plone.org/mailman/listinfo/framework-team
Re: [Framework-Team] PLIP #247 ready for review (I think)
On Sat, Jan 17, 2009 at 02:37, Ethan Jucovy ethan.juc...@gmail.com wrote: I've implemented PLIP #247, Automate ZCML Loading for Plone Plugins (http://plone.org/products/plone/roadmap/247) and committed a review buildout here: https://svn.plone.org/svn/plone/review/plip247-automate-plugin-zcml for your review. I have started my review. To my dismay, however, the z3c.autoinclude package has several test failures. I understand that this package is also being used by the Grok project, and it appears to work as advertised, but test failures is not a good sign of package quality. For example, utils.txt somehow triggers an assert on line 99 in utils.py, which has a somewhat distressing comment: assert len(non_namespaced_dists) == 1 ### we really are in trouble if we get into a situation with more than one Could you comment on the state of these failures? The buildout is branched from the 3.3 review bundle buildout based on buildout.eggtractor, which automatically generates ZCML slugs for development packages and puts them in parts/instance/etc/package- includes. Since the purpose of PLIP 247 is to automate ZCML loading, those automatically generated ZCML slugs should be removed. I've hardly ever used buildout so I'm not sure how to change that behavior -- sorry about that; I'll change it if I can figure out how. Simply *not* use buildout.eggtractor; you only have 3 eggs in this buildout, add them to the develop, eggs and zcml lines and Bob's your uncle. Alternatively, just set tractor-autoload-zcml to false, which you did. -- Martijn Pieters ___ Framework-Team mailing list Framework-Team@lists.plone.org http://lists.plone.org/mailman/listinfo/framework-team
Re: [Framework-Team] PLIP #247 ready for review (I think)
On Sat, Jan 17, 2009 at 02:37, Ethan Jucovy ethan.juc...@gmail.com wrote: I've implemented PLIP #247, Automate ZCML Loading for Plone Plugins (http://plone.org/products/plone/roadmap/247) and committed a review buildout here: https://svn.plone.org/svn/plone/review/plip247-automate-plugin-zcml for your review. Here is my full verdict, as committed to the bundle readme: I've updated the buildout to not use buildout.eggtractor; you already had disabled it's zcml autoloading but didn't add Products.CMFPlone to the zcml line. Not using buildout.eggtractor at all is a better idea here, however. I've also added a foo.cfg buildout configuration to test the (very nice!) demonstration package. Just run bin/buildout -c foo.cfg to have the foo package included. After these changes, the z3c.autoinclude package appears to work as advertised. However, when running the z3c.autoinclude tests I see test failures, which worries me. I understand that the package is being used by Grok as well, and this PLIP merely covers the inclusion of the package into Plone, but I'd like to see some comments about what these test failures are about. Until I see comments that alleviate my concerns about the test failures, I'm giving this PLIP a +0. -- Martijn Pieters ___ Framework-Team mailing list Framework-Team@lists.plone.org http://lists.plone.org/mailman/listinfo/framework-team
Re: [Framework-Team] PLIP #247 ready for review (I think)
On Sat, Jan 17, 2009 at 2:37 AM, Ethan Jucovy ethan.juc...@gmail.comwrote: Some of the things that people have said ought to be discussed about PLIP #247 are: 1) impact on server startup time 2) implications for test environments 3) stabilizing and releasing a new version of z3c.autoinclude 4) debugging tools/APIs for z3c.autoinclude * 5) documentation and/or code generation templates to explain the entry point for plugin authors* I'll begin working on #3 immediately, since there's plenty I can do there while discussion is still happening. Please let me know if there's anything else I'll need to submit for the PLIP's review. I'm not a member of the Framework Team so this is not mandatory for the PLIP's review, but as a member of the Documentation Team I would appreciate number (5). Something like Before, we did this to accomplish this task. Now, we can do that., as Wichert (?) suggested, so we would be able to find and update affected docs easily. Thanks in advance! -- israel ___ Framework-Team mailing list Framework-Team@lists.plone.org http://lists.plone.org/mailman/listinfo/framework-team
