I thought this might be intresting for discussion on zope-dev as well... /dario ----- Original Message ----- From: "Andrew Kenneth Milton" <akm@theinternet.com.au> To: "Andrew Kenneth Milton" <akm@theinternet.com.au> Cc: <exuserfolder-devel@sourceforge.net> Sent: Thursday, November 29, 2001 8:45 AM Subject: Re: [Exuserfolder-devel] Zope 2.5b1 release It has been pointed out that perhaps the dripping sarcasm drowned out the acutal points of the email, sorry I was particularly pissed off at how half-assed it all was. If you are writing a product that needs to create a user, you go look in User.py and find out what the API since that tends to be the only place the API is documented. If you look at how UserFolder is implemented; a) The change to manage_* seems to be completely arbitrary, since we already had _do* methods that meant you didn't have to call manage_users with fake submit buttons. So what is the point of having manage_ ? b) manage_* methods usually indicate web callable methods, which these clearly aren't (no docstring, no REQUEST parameter). In fact they don't return a status at all, so they seem to doubly useless. c) If it's an internal API for applications then the methods should probably be protected by being prefixed with an _ to indicate they're not for calling from the web or from within Documents/Templates. d) Even if you conformed to the API by calling e.g. manage_addUser() this seemed to be incorrect, since it didn't do things like encrypt the password. This meant you would have to either a) encrypt the password yourself (*very bad for XUF*), or b) call _addUser() which is not part of a defined API but looks like it should be (i.e. manage_addUser and _addUser seem to be reversed in functionality). ii) This is compounded by the fact you would have to query the UserFolder itself to find out if you should be encrypting passwords (what no UserFolder capability API?). e) s/_do/manage_/ doesn't constitute a new API. It's just the old one with the names changed. f) We've done a lot of work to make a flexible UF product that is I18Ned (well -able), and they still use the value of the submit button. I would have expected to see an alternative manage_users called from the ZMI that behaved much better, with manage_users calls raising warnings. g) Someone got paid to do it, and that just pisses me off, given the quality of the stuff we release for free (and I'm still looking for a job btw d8). We're not perfect, the 0.10.1 release shows that, but, at least we spent more than three minutes including thinking time on it, and I at least try not to change things just for the sake of it. Anyway. In case you were wondering what the previous email meant, there's the actual meaning d8) -- Totally Holistic Enterprises Internet| | Andrew Milton The Internet (Aust) Pty Ltd | | ACN: 082 081 472 ABN: 83 082 081 472 | M:+61 416 022 411 | Carpe Daemon PO Box 837 Indooroopilly QLD 4068 |akm@theinternet.com.au| _______________________________________________ Exuserfolder-devel mailing list Exuserfolder-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/exuserfolder-devel
Dario Lopez-Kästen wrote:
Anyway. In case you were wondering what the previous email meant, there's the actual meaning d8)
I'm sure Andy's got a point, but we haven't got the context of those points. Is he complaining about the change in UserFolder API in Zope 2.5? cheers, Chris
Whew, that email (and the preceding one in the thread) is quite a whopper. In substance, amk raises some pretty serious issues that we need to come to grips with very quickly. I don't have enough information to respond right now, but trust that we'll get a good response back today. Neo-moderator note: if the discussion _does_ start over here, everyone is advised to leave the anger at the door. Problems like this can usually be solved pretty easily by direct communication and open minds. --Paul Dario Lopez-Kästen wrote:
I thought this might be intresting for discussion on zope-dev as well...
/dario
----- Original Message ----- From: "Andrew Kenneth Milton" <akm@theinternet.com.au> To: "Andrew Kenneth Milton" <akm@theinternet.com.au> Cc: <exuserfolder-devel@sourceforge.net> Sent: Thursday, November 29, 2001 8:45 AM Subject: Re: [Exuserfolder-devel] Zope 2.5b1 release
It has been pointed out that perhaps the dripping sarcasm drowned out the acutal points of the email, sorry I was particularly pissed off at how half-assed it all was.
If you are writing a product that needs to create a user, you go look in User.py and find out what the API since that tends to be the only place the API is documented.
If you look at how UserFolder is implemented;
a) The change to manage_* seems to be completely arbitrary, since we already had _do* methods that meant you didn't have to call manage_users with fake submit buttons. So what is the point of having manage_ ?
b) manage_* methods usually indicate web callable methods, which these clearly aren't (no docstring, no REQUEST parameter). In fact they don't return a status at all, so they seem to doubly useless.
c) If it's an internal API for applications then the methods should probably be protected by being prefixed with an _ to indicate they're not for calling from the web or from within Documents/Templates.
d) Even if you conformed to the API by calling e.g. manage_addUser() this seemed to be incorrect, since it didn't do things like encrypt the password. This meant you would have to either a) encrypt the password yourself (*very bad for XUF*), or b) call _addUser() which is not part of a defined API but looks like it should be (i.e. manage_addUser and _addUser seem to be reversed in functionality).
ii) This is compounded by the fact you would have to query the UserFolder itself to find out if you should be encrypting passwords (what no UserFolder capability API?).
e) s/_do/manage_/ doesn't constitute a new API. It's just the old one with the names changed.
f) We've done a lot of work to make a flexible UF product that is I18Ned (well -able), and they still use the value of the submit button. I would have expected to see an alternative manage_users called from the ZMI that behaved much better, with manage_users calls raising warnings.
g) Someone got paid to do it, and that just pisses me off, given the quality of the stuff we release for free (and I'm still looking for a job btw d8). We're not perfect, the 0.10.1 release shows that, but, at least we spent more than three minutes including thinking time on it, and I at least try not to change things just for the sake of it.
Anyway. In case you were wondering what the previous email meant, there's the actual meaning d8)
-- Totally Holistic Enterprises Internet| | Andrew Milton The Internet (Aust) Pty Ltd | | ACN: 082 081 472 ABN: 83 082 081 472 | M:+61 416 022 411 | Carpe Daemon PO Box 837 Indooroopilly QLD 4068 |akm@theinternet.com.au|
_______________________________________________ Exuserfolder-devel mailing list Exuserfolder-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/exuserfolder-devel
_______________________________________________ Zope-Dev maillist - Zope-Dev@zope.org http://lists.zope.org/mailman/listinfo/zope-dev ** No cross posts or HTML encoding! ** (Related lists - http://lists.zope.org/mailman/listinfo/zope-announce http://lists.zope.org/mailman/listinfo/zope )
Yikes, it was pointed out to me that I typed "amk" instead of "akm". Though I knew it was Andrew Milton and not Andrew Kuchling, I mixed up the letters. (In fact, as I was typing it, I was thinking "wow, that looks like Andrew Kuchling's monogram.") --Paul Paul Everitt wrote:
Whew, that email (and the preceding one in the thread) is quite a whopper. In substance, amk raises some pretty serious issues that we need to come to grips with very quickly.
I don't have enough information to respond right now, but trust that we'll get a good response back today.
Neo-moderator note: if the discussion _does_ start over here, everyone is advised to leave the anger at the door. Problems like this can usually be solved pretty easily by direct communication and open minds.
--Paul
Dario Lopez-Kästen wrote:
I thought this might be intresting for discussion on zope-dev as well...
/dario
----- Original Message ----- From: "Andrew Kenneth Milton" <akm@theinternet.com.au> To: "Andrew Kenneth Milton" <akm@theinternet.com.au> Cc: <exuserfolder-devel@sourceforge.net> Sent: Thursday, November 29, 2001 8:45 AM Subject: Re: [Exuserfolder-devel] Zope 2.5b1 release
It has been pointed out that perhaps the dripping sarcasm drowned out the acutal points of the email, sorry I was particularly pissed off at how half-assed it all was.
If you are writing a product that needs to create a user, you go look in User.py and find out what the API since that tends to be the only place the API is documented.
If you look at how UserFolder is implemented;
a) The change to manage_* seems to be completely arbitrary, since we already had _do* methods that meant you didn't have to call manage_users with fake submit buttons. So what is the point of having manage_ ?
b) manage_* methods usually indicate web callable methods, which these clearly aren't (no docstring, no REQUEST parameter). In fact they don't return a status at all, so they seem to doubly useless.
c) If it's an internal API for applications then the methods should probably be protected by being prefixed with an _ to indicate they're not for calling from the web or from within Documents/Templates.
d) Even if you conformed to the API by calling e.g. manage_addUser() this seemed to be incorrect, since it didn't do things like encrypt the password. This meant you would have to either a) encrypt the password yourself (*very bad for XUF*), or b) call _addUser() which is not part of a defined API but looks like it should be (i.e. manage_addUser and _addUser seem to be reversed in functionality).
ii) This is compounded by the fact you would have to query the UserFolder itself to find out if you should be encrypting passwords (what no UserFolder capability API?).
e) s/_do/manage_/ doesn't constitute a new API. It's just the old one with the names changed.
f) We've done a lot of work to make a flexible UF product that is I18Ned (well -able), and they still use the value of the submit button. I would have expected to see an alternative manage_users called from the ZMI that behaved much better, with manage_users calls raising warnings.
g) Someone got paid to do it, and that just pisses me off, given the quality of the stuff we release for free (and I'm still looking for a job btw d8). We're not perfect, the 0.10.1 release shows that, but, at least we spent more than three minutes including thinking time on it, and I at least try not to change things just for the sake of it.
Anyway. In case you were wondering what the previous email meant, there's the actual meaning d8)
-- Totally Holistic Enterprises Internet| | Andrew Milton The Internet (Aust) Pty Ltd | | ACN: 082 081 472 ABN: 83 082 081 472 | M:+61 416 022 411 | Carpe Daemon PO Box 837 Indooroopilly QLD 4068 |akm@theinternet.com.au|
_______________________________________________ Exuserfolder-devel mailing list Exuserfolder-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/exuserfolder-devel
_______________________________________________ Zope-Dev maillist - Zope-Dev@zope.org http://lists.zope.org/mailman/listinfo/zope-dev ** No cross posts or HTML encoding! ** (Related lists - http://lists.zope.org/mailman/listinfo/zope-announce http://lists.zope.org/mailman/listinfo/zope )
_______________________________________________ Zope-Dev maillist - Zope-Dev@zope.org http://lists.zope.org/mailman/listinfo/zope-dev ** No cross posts or HTML encoding! ** (Related lists - http://lists.zope.org/mailman/listinfo/zope-announce http://lists.zope.org/mailman/listinfo/zope )
A K Milton wrote:
a) The change to manage_* seems to be completely arbitrary, since we already had _do* methods that meant you didn't have to call manage_users with fake submit buttons. So what is the point of having manage_ ?
They were added in response to this fishbowl proposal: http://dev.zope.org/Wikis/DevSite/Proposals/UserFolderXmlRpcQuickFix It was a quick fix intended to help people doing user management over XML-RPC. If there are problems in maintaining compatibility with the previous API, and products that rely on that, well that's a bug and it needs Collecting and sorting out before 2.5 final. I'm concerned about this too, and I'm glad it's reached Zope-Dev, as I've got some LoginManager user folders in use, and I don't want these to break when I start using Zope 2.5 on those systems. In the fishbowl proposal comments, Brial Lloyd wrote: ---- This is so long overdue that I've just checked this in for the Zope 2.5 line in CVS. (It is slated for 2.5 because it is an API change and has documentation impact, plus I would like to follow up and clean up some of the old form dispatch code and want to make sure we have an upgrade cycle to make sure other implementations of user folders don't break). ---- I see an intention not to break other user folder products. Given that the fishbowl proposal in question is supposed to make for a very small change, any breakage in existing products is a bug in its implementation. In summary: I want to make sure that things are no worse in Zope 2.5 final than in Zope 2.5. Any breakage caused by this API change is a bug, and needs to be sorted out by Zope 2.5 final. I can offer some help in fixing these bugs, especially if they find their way into the Collector, so I can take ownership of them. Improvements to the user folder API that fall outside "getting it working with XML-RPC" bring up larger issues, which I see are being discussed here: http://dev.zope.org/Wikis/DevSite/Proposals/BetterUserManagement -- Steve Alexander Software Engineer Cat-Box limited
Steve Alexander wrote:
In summary:
I want to make sure that things are no worse in Zope 2.5 final than in Zope 2.5. Any breakage caused by this API change is a bug, and needs to be sorted out by Zope 2.5 final.
That should have read: ---- I want to make sure that the user management API is no worse in Zope 2.5 than in Zope 2.4. ---- -- Steve Alexander
a) The change to manage_* seems to be completely arbitrary, since we already had _do* methods that meant you didn't have to call manage_users with fake submit buttons. So what is the point of having manage_ ?
They were added in response to this fishbowl proposal:
http://dev.zope.org/Wikis/DevSite/Proposals/UserFolderXmlRpcQuickFix
It was a quick fix intended to help people doing user management over XML-RPC.
Right - the idea was to _add to_ the existing API, in a completely backward-compatible way, so that: - "untrusted code" (DTML, Python scripts, other code managed by security constraints) could be used to do user mgmt (if the caller has appropriate rights, of course). Previously, the only code accessible to these was unwieldy (the build-a-fake- request approach), and the corresponding "_" methods were not accessible because "_" methods can't be called from Web code. - "trusted code" (external methods, Python products) would have a clearer and easier API for doing the same. While they could have used the "_" methods, they are not documented as if they are a part of the official API, which is a point of confusion. - An XML-RPC (given appropriate rights) call could be used to do user management work.
If there are problems in maintaining compatibility with the previous API, and products that rely on that, well that's a bug and it needs Collecting and sorting out before 2.5 final.
I'm concerned about this too, and I'm glad it's reached Zope-Dev, as I've got some LoginManager user folders in use, and I don't want these to break when I start using Zope 2.5 on those systems.
Nor do I - my goal for this was (and remains) 100% backward compatibility, and to make people's lives easier, not harder. It looks like I've muffed the implementation, for which I certainly take full responsibility (and which I'll rectify today). I think I also failed to adequately express the goal for this - I'll need to update the docstrings as well. The goal was not to change the (admitted ancient and crummy) way that the Web interfaces to user folders interact with the API (the dispatching based on submit button lameness), as I'm sure that many, many implementations would break. The goal was not to deprecate _that_ usage of the 'manage_users' method. The idea was to deprecate the use of 'manage_users' from Web-based or product code in favor of the new (and hopefully easier) APIs. That allows us to address the lameness of the 'manage_users' method re: user folder UI as a separate issue in the future, while still being able to give "scripters", product authors and XML-RPC users something that they can use now. Brian Lloyd brian@zope.com Software Engineer 540.361.1716 Zope Corporation http://www.zope.com
I see an intention not to break other user folder products. Given that the fishbowl proposal in question is supposed to make for a very small change, any breakage in existing products is a bug in its implementation.
I've just checked in changes that I believe address all of the issues (there were several): - Part of the problem is that I picked really poor names for the added APIs (names that had a high likelyhood of, and in fact were used already by custom user folder implementations). I've changed them to 'userFolderAddUser', 'userFolderEditUser' and 'userFolderDelUsers', making the names somewhat uglier and less likely to clash. - They really only need to act as permission-protected aliases to the methods that custom user folders already implement ('_doAddUser', '_doChangeUser', '_doDelUsers'). I've done that, and custom user folder authors don't need to take any action (other than have implemented the '_' methods in the first place). - Password encryption was being done in the wrong place. It really wants to be done in the _doAddUser and _doChangeUser, so that custom user folders can elect to do it or not (since some pw schemes cannot work with a pre-encrypted password). I've changed it so that the built-in Zope user folder will do encryption, and custom user folders can support it easily by changing their _doAddUser and _doChangeUser to do it if appropriate. - Updated all of the comments to (hopefully) remove confusion about 'deprecation'. So the current status is that: - Current user folders should continue to work without any changes, and they don't need to do anything to support the new userFolder* convenience APIs. - Nothing is really "deprecated" - I've changed the wording to say that scripts and other web-based code are encouraged to use the new userFolder* APIs to work with users rather than create more code dependent on the crummy 'manage_users' hackery. I've tested this on a few variations here, but I'd be happy if some other folks who have alternate user folders around could give the updated lib/python/AccessControl/User.py a whirl and let me know about any problems. Brian Lloyd brian@zope.com Software Engineer 540.361.1716 Zope Corporation http://www.zope.com
participants (5)
-
Brian Lloyd -
Chris Withers -
Dario Lopez-Kästen -
Paul Everitt -
Steve Alexander