Re: [ZODB-Dev] breaking out the transaction module from ZODB
On Nov 8, 2007, at 7:46 PM, Chris McDonough wrote: How about zope.transaction? Guido recently told me that people in the Python community at large assume that anything in the Zope namespace is assumed to be Zope specific, so I'd rather not put it there. Does it matter? People who are allergic to the name zope can probably lose. It maters to me. There's a good deal of 3rd-party code that does import transaction Good point. I guess we should leave the package where it is. Note that then we have a tricky issue with avoiding having the package installed twice. I guess we should ignore this for now. :/ It'd be no problem to provide the shims. I don't agree. Shims and similar tricks are evil. Sometimes, it's a necessary evil, but I don't think the case is strong enough here. Jim -- Jim Fulton Zope Corporation ___ For more information about ZODB, see the ZODB Wiki: http://www.zope.org/Wikis/ZODB/ ZODB-Dev mailing list - ZODB-Dev@zope.org http://mail.zope.org/mailman/listinfo/zodb-dev
Re: [ZODB-Dev] breaking out the transaction module from ZODB
On Nov 9, 2007, at 7:52 AM, Jim Fulton wrote: On Nov 8, 2007, at 7:46 PM, Chris McDonough wrote: How about zope.transaction? Guido recently told me that people in the Python community at large assume that anything in the Zope namespace is assumed to be Zope specific, so I'd rather not put it there. Does it matter? People who are allergic to the name zope can probably lose. It maters to me. OK. I defer here. But in general, I think what would probably work better than a new z namespace or any other avoidance of the zope name is is better dependency specifications, so people would feel better about trying to easy_install zope.* packages. Tres suggested yesterday that we should write a buildbot-like thing that checked out each top-level package in SVN and installed it into a fresh virtualenv to see what its dependencies actually are and fix the too-conservative dependencies. I've made a 'zope.transaction' package that I'll rename to 'transaction': one test still fails in its current state, which I should get fixed today: http://svn.zope.org/zope.transaction/trunk/ WeakSet is in weakset.py. It also contains TimeStamp, which will get moved out of persistent. The tests work (reqt's are downloaded) if you do setup.py test -q - C ___ For more information about ZODB, see the ZODB Wiki: http://www.zope.org/Wikis/ZODB/ ZODB-Dev mailing list - ZODB-Dev@zope.org http://mail.zope.org/mailman/listinfo/zodb-dev
Re: [ZODB-Dev] breaking out the transaction module from ZODB
On Nov 9, 2007, at 8:31 AM, Chris McDonough wrote: It also contains TimeStamp, which will get moved out of persistent. Why? I don't see any uses of TimeStamp by the transaction package. In your new package, it is only used by its tests. The tests work (reqt's are downloaded) if you do setup.py test -q Yawn. IMO, the test command in setuptools is a waste of time, because it doesn't work with anything else. zope.interface is a real requirement. It is already in test_requires. Over time, we need to clean up the transaction tests so they don't use ZODB. Jim -- Jim Fulton Zope Corporation ___ For more information about ZODB, see the ZODB Wiki: http://www.zope.org/Wikis/ZODB/ ZODB-Dev mailing list - ZODB-Dev@zope.org http://mail.zope.org/mailman/listinfo/zodb-dev
Re: [ZODB-Dev] breaking out the transaction module from ZODB
On Nov 9, 2007, at 8:41 AM, Jim Fulton wrote: On Nov 9, 2007, at 8:31 AM, Chris McDonough wrote: It also contains TimeStamp, which will get moved out of persistent. Why? I don't see any uses of TimeStamp by the transaction package. In your new package, it is only used by its tests. D'oh! You're right. Out it goes. I wrote tests for it, I'll add them to the persistent package. The tests work (reqt's are downloaded) if you do setup.py test -q Yawn. IMO, the test command in setuptools is a waste of time, because it doesn't work with anything else. It runs all the tests, even the doctests, if thats what you mean. See the additional_tests hair in the test modules. zope.interface is a real requirement. It is already in test_requires. Over time, we need to clean up the transaction tests so they don't use ZODB. Yeah, given that we're name this thing transaction, it's an actually an immediate requirement. There's only one test that uses anything that can't be mocked up in the transaction package (it uses an actual MappingStorage and a DB) itself. It also happens to be the one that fails right now; I haven't tried to understand it yet. - C ___ For more information about ZODB, see the ZODB Wiki: http://www.zope.org/Wikis/ZODB/ ZODB-Dev mailing list - ZODB-Dev@zope.org http://mail.zope.org/mailman/listinfo/zodb-dev
Re: [ZODB-Dev] breaking out the transaction module from ZODB
On Nov 9, 2007, at 9:29 AM, Chris McDonough wrote: On Nov 9, 2007, at 8:41 AM, Jim Fulton wrote: On Nov 9, 2007, at 8:31 AM, Chris McDonough wrote: It also contains TimeStamp, which will get moved out of persistent. Why? I don't see any uses of TimeStamp by the transaction package. In your new package, it is only used by its tests. D'oh! You're right. Out it goes. I wrote tests for it, I'll add them to the persistent package. Thanks! The tests work (reqt's are downloaded) if you do setup.py test -q Yawn. IMO, the test command in setuptools is a waste of time, because it doesn't work with anything else. It runs all the tests, even the doctests, if thats what you mean. See the additional_tests hair in the test modules. My point is that the meta data you added to the setup.py only works with setup.py. It isn't accessible to any other test runners. I shouldn't have yawned. It is significant that the tests work. :) I just don't find this way of running the tests to be useful. The first time I work on this package, I'll add a buildout.cfg so I can use the Zope test runner. At that point, I'll have to deal with these extra requirements in another way (which is no big deal). zope.interface is a real requirement. It is already in test_requires. Over time, we need to clean up the transaction tests so they don't use ZODB. Yeah, given that we're name this thing transaction, it's an actually an immediate requirement. If you are going to spend the time, then, uh, sure. ;) Seriously, while I would love to see this cleaned up, I don't think I would consider this super urgent. I guess that depends on competing priorities. There's only one test that uses anything that can't be mocked up in the transaction package (it uses an actual MappingStorage and a DB) itself. It also happens to be the one that fails right now; I haven't tried to understand it yet. Gah. BTW, if you haven't already, you should check for transaction tests lurking in the other ZODB packages. Jim -- Jim Fulton Zope Corporation ___ For more information about ZODB, see the ZODB Wiki: http://www.zope.org/Wikis/ZODB/ ZODB-Dev mailing list - ZODB-Dev@zope.org http://mail.zope.org/mailman/listinfo/zodb-dev
Re: [ZODB-Dev] breaking out the transaction module from ZODB
On Nov 9, 2007, at 9:29 AM, Chris McDonough wrote: On Nov 9, 2007, at 8:41 AM, Jim Fulton wrote: On Nov 9, 2007, at 8:31 AM, Chris McDonough wrote: It also contains TimeStamp, which will get moved out of persistent. Why? I don't see any uses of TimeStamp by the transaction package. In your new package, it is only used by its tests. D'oh! You're right. Out it goes. I wrote tests for it, I'll add them to the persistent package. BTW, I doubt that persistent is the right location for TimeStamp either, but that's a different project. TimeStamp *is* there now, so that's the right place to put these tests (now). Someday, I'm going to refactor persistent in a pretty major way. Jim -- Jim Fulton Zope Corporation ___ For more information about ZODB, see the ZODB Wiki: http://www.zope.org/Wikis/ZODB/ ZODB-Dev mailing list - ZODB-Dev@zope.org http://mail.zope.org/mailman/listinfo/zodb-dev
Re: [ZODB-Dev] breaking out the transaction module from ZODB
On Nov 9, 2007, at 9:43 AM, Jim Fulton wrote: Yawn. IMO, the test command in setuptools is a waste of time, because it doesn't work with anything else. It runs all the tests, even the doctests, if thats what you mean. See the additional_tests hair in the test modules. My point is that the meta data you added to the setup.py only works with setup.py. It isn't accessible to any other test runners. I shouldn't have yawned. It is significant that the tests work. :) I just don't find this way of running the tests to be useful. The first time I work on this package, I'll add a buildout.cfg so I can use the Zope test runner. At that point, I'll have to deal with these extra requirements in another way (which is no big deal). What if we caused setup.py to read a buildout.cfg for the tests_require package names and we passed these in as tests_require= names? Would that make it all better? zope.interface is a real requirement. It is already in test_requires. Over time, we need to clean up the transaction tests so they don't use ZODB. Yeah, given that we're name this thing transaction, it's an actually an immediate requirement. If you are going to spend the time, then, uh, sure. ;) Seriously, while I would love to see this cleaned up, I don't think I would consider this super urgent. I guess that depends on competing priorities. I just deleted the sections of the test_transaction doctests that depended on ZODB. They were actually not really testing transactions, they were testing persistent object behavior. I'll try to put them back in a form within ZODB proper, as the test really are testing ZODB functionality, not transaction functionality. There's only one test that uses anything that can't be mocked up in the transaction package (it uses an actual MappingStorage and a DB) itself. It also happens to be the one that fails right now; I haven't tried to understand it yet. Gah. BTW, if you haven't already, you should check for transaction tests lurking in the other ZODB packages. Good idea. In the meantime, I've gotten rid of 'zope.transaction' and I've created a new top-level 'transaction' package at http://svn.zope.org/transaction/ . All its tests pass. It depends only on 'zope.interface', and requires 'zope.testing' for running the tests. - C ___ For more information about ZODB, see the ZODB Wiki: http://www.zope.org/Wikis/ZODB/ ZODB-Dev mailing list - ZODB-Dev@zope.org http://mail.zope.org/mailman/listinfo/zodb-dev
Re: [ZODB-Dev] breaking out the transaction module from ZODB
On Nov 9, 2007, at 11:37 AM, Chris McDonough wrote: On Nov 9, 2007, at 9:43 AM, Jim Fulton wrote: Yawn. IMO, the test command in setuptools is a waste of time, because it doesn't work with anything else. It runs all the tests, even the doctests, if thats what you mean. See the additional_tests hair in the test modules. My point is that the meta data you added to the setup.py only works with setup.py. It isn't accessible to any other test runners. I shouldn't have yawned. It is significant that the tests work. :) I just don't find this way of running the tests to be useful. The first time I work on this package, I'll add a buildout.cfg so I can use the Zope test runner. At that point, I'll have to deal with these extra requirements in another way (which is no big deal). What if we caused setup.py to read a buildout.cfg for the tests_require package names and we passed these in as tests_require= names? Would that make it all better? Too complicated. :) Again, this isn't a big deal. You've done the hard work of figuring out what's required. You even reduced the requirements. I can't ask for more. In fact, from what you've written below, nothing there aren't any extra requirements if the Zope test runner is used, as it will already cause zope.testing to be required. zope.interface is a real requirement. It is already in test_requires. Over time, we need to clean up the transaction tests so they don't use ZODB. Yeah, given that we're name this thing transaction, it's an actually an immediate requirement. If you are going to spend the time, then, uh, sure. ;) Seriously, while I would love to see this cleaned up, I don't think I would consider this super urgent. I guess that depends on competing priorities. I just deleted the sections of the test_transaction doctests that depended on ZODB. They were actually not really testing transactions, they were testing persistent object behavior. I'll try to put them back in a form within ZODB proper, as the test really are testing ZODB functionality, not transaction functionality. Way cool. There's only one test that uses anything that can't be mocked up in the transaction package (it uses an actual MappingStorage and a DB) itself. It also happens to be the one that fails right now; I haven't tried to understand it yet. Gah. BTW, if you haven't already, you should check for transaction tests lurking in the other ZODB packages. Good idea. In the meantime, I've gotten rid of 'zope.transaction' and I've created a new top-level 'transaction' package at http:// svn.zope.org/transaction/ . All its tests pass. It depends only on 'zope.interface', and requires 'zope.testing' for running the tests. Yay! I think you are pretty close to done -- if not done. Much thanks. I wish there was a way to state anti-requirements in setuptools. Then we could say that transaction had an anti-requirement for ZODB3 3.9. BTW, it would be nice to now remove the transaction package from the ZODB trunk and make it a dependency. Jim -- Jim Fulton Zope Corporation ___ For more information about ZODB, see the ZODB Wiki: http://www.zope.org/Wikis/ZODB/ ZODB-Dev mailing list - ZODB-Dev@zope.org http://mail.zope.org/mailman/listinfo/zodb-dev
Re: [ZODB-Dev] breaking out the transaction module from ZODB
On Nov 9, 2007, at 12:10 PM, Chris McDonough wrote: In the meantime, I've gotten rid of 'zope.transaction' and I've created a new top-level 'transaction' package at http:// svn.zope.org/transaction/ . All its tests pass. It depends only on 'zope.interface', and requires 'zope.testing' for running the tests. Yay! I think you are pretty close to done -- if not done. Much thanks. I wish there was a way to state anti-requirements in setuptools. Then we could say that transaction had an anti-requirement for ZODB3 3.9. Yeah, especially given that I removed a deprecated method (beforeCommitHook), so its installation may tend to break running systems. Maybe I should put it back, as systems will probably continue to just work even if they have this installed for any system that uses a recent Zope. I'm not sure what's best here. One option might be to write something that searches sys.path looking for and complaining about multiple transaction modules. BTW, it would be nice to now remove the transaction package from the ZODB trunk and make it a dependency. Yes. ZODB's setup.py is polyglotic... it works if setuptools isn't installed. I suspect it shouldn't continue to given that it now has an external egg dependency. My intent, if I can find time, is to rewrite the setup script from scratch and require setuptools. I suspect it will be much simpler at that point. Jim -- Jim Fulton Zope Corporation ___ For more information about ZODB, see the ZODB Wiki: http://www.zope.org/Wikis/ZODB/ ZODB-Dev mailing list - ZODB-Dev@zope.org http://mail.zope.org/mailman/listinfo/zodb-dev
Re: [ZODB-Dev] breaking out the transaction module from ZODB
On Nov 9, 2007, at 12:10 PM, Chris McDonough wrote: BTW, it would be nice to now remove the transaction package from the ZODB trunk and make it a dependency. Yes. ZODB's setup.py is polyglotic... it works if setuptools isn't installed. I suspect it shouldn't continue to given that it now has an external egg dependency. I've made changes to the ZODB setup.py and I've remove the 'transaction' directory from ZODB/src. The changes also imply that setuptools is required to run setup.py, and the 'transaction' distribution is named as an install_requires dependency. Setuptools is now required to install the ZODB head. I made a tag before I did this at http://svn.zope.org/ZODB/tags/before_transaction_remove/ in case I hosed anything in the process. - C ___ For more information about ZODB, see the ZODB Wiki: http://www.zope.org/Wikis/ZODB/ ZODB-Dev mailing list - ZODB-Dev@zope.org http://mail.zope.org/mailman/listinfo/zodb-dev
Re: [ZODB-Dev] breaking out the transaction module from ZODB
On Nov 7, 2007, at 11:38 PM, Chris McDonough wrote: I've begun work on breaking out the transaction module so it can be used independently from ZODB. Great! Here's what I've done so far: - I've moved TransactionError and TransactionFailedError from ZODB.POSException into transaction.interfaces, e.g.: class TransactionError(Exception): An error occurred due to normal transaction processing. +1 class TransactionFailedError(Exception): Cannot perform an operation on a transaction that previously failed. An attempt was made to commit a transaction, or to join a transaction, but this transaction previously raised an exception during an attempt to commit it. The transaction must be explicitly aborted, either by invoking abort() on the transaction, or begin() on its transaction manager. Why not subclass TransactionError? - I've caused ZODB.POSException to add the POSError base class to both TransactionError and TransactionFailedError after importing them from transaction.interfaces, e.g.: from transaction.interfaces import TransactionError from transaction.interfaces import TransactionFailedError # We want to be able to distribute the transaction module independent # from ZODB but we need to maintain backwards compatibility with older # ZODB releases, where TransactionError and TransactionFailedError # were actually defined within ZODB.POSException, and inherited from # POSError. With this solution, if ZODB is present, TransactionError # and TransactionFailedError will have POSError as a base class. If # ZODB is not present, they won't. Thanks to Ian Bicking for # suggesting this solution; as ugly as it is, it does the job. TransactionError.__bases__ += (POSError,) TransactionFailedError.__bases__ += (POSError,) Is this *really* necessary? It's obviously a bit evil. Let's explore alternatives to this: 1. Just don't do it. I'd be a bit surprised if there was code actually catching POSError. 2,. If we really (really really) need to support catching POSError, then perhaps we should mobe POSError to the transaction package. - I've created a zc.zodbutils package that is essentially the code that currently lives in the ZODB.utils module; I've also moved the TimeStamp.c code that currently lives in 'persistent' into it. A stub ZODB.utils module exists that just does from zc.zodbutils import *, and in the persistent package's __init__.py, I do from zc.zodbutils import TimeStamp for backwards compatibility. I'd rather not do this. Let's be a bit more selective here. The number of imports from ZODB are pretty limited. Many of them should move to transaction. Some of them are just test utilities that can be duplicated. I think the biggest challenge is WeakSet. This could be broken out into a separate package, but I think it's not as general as its name implies and should probably just be moved to transaction. The intention is that the transaction distribution will depend only on zc.zodbutils (as will of course the ZODB distro, along with its other current dependencies plus the transaction distribution). I think this is overly complicated. Let's just move or copy a few things to transaction. I'm wondering about version numbering and naming for the separate packages.. I suspect we shouldn't try to marry the transaction distribution version number to the ZODB distribution version number because they really won't be tied together that way. Agreed, Maybe just start transaction at 1.0 or something. Yup. And I'm thinking that the transaction distribution should be named just transaction. Yes, unless we decide to move the package. I think transaction is a bit presumptuous. :) There is a more important issue that also suggests that moving the package. A potential danger with distutils is that different distributions can provide the same Python package, which is a recipe for extreme confusion at best. Imagine someone installing transaction 1.0 and ZODB 3.8. I'd be *very* tempted to start a z namespace package (as I wish I'd done a long time ago :) and put it there. Granted that claiming z is also a bit presumptuous, but I think we'd have a reasonable clam to it. :) Moving the package avoids accidentally installing 2 transaction modules at the same time. And the name zc.zodbutils is just a placeholder, suggestions from interested parties would be helpful. Let's not even do this. See above, I haven't adjusted any imports in tests, nor have I repackaged the transaction module using setuptools yet. I wanted to get a sense of whether folks thought what I've done so far is reasonable or if you might have done it differently. Thanks for working on this. See my comments above. Jim -- Jim Fulton Zope Corporation ___ For
Re: [ZODB-Dev] breaking out the transaction module from ZODB
On Nov 8, 2007, at 9:14 AM, Jim Fulton wrote: class TransactionFailedError(Exception): Cannot perform an operation on a transaction that previously failed. An attempt was made to commit a transaction, or to join a transaction, but this transaction previously raised an exception during an attempt to commit it. The transaction must be explicitly aborted, either by invoking abort() on the transaction, or begin() on its transaction manager. Why not subclass TransactionError? It didn't before. Should it? There's also a DoomedTransaction exception in the interfaces package that could. TransactionError.__bases__ += (POSError,) TransactionFailedError.__bases__ += (POSError,) Is this *really* necessary? It's obviously a bit evil. Let's explore alternatives to this: 1. Just don't do it. I'd be a bit surprised if there was code actually catching POSError. Me too; +1. If I notice anything relying on them inheriting from POSError, I'll move POSError into the transaction module. - I've created a zc.zodbutils package that is essentially the code that currently lives in the ZODB.utils module; I've also moved the TimeStamp.c code that currently lives in 'persistent' into it. A stub ZODB.utils module exists that just does from zc.zodbutils import *, and in the persistent package's __init__.py, I do from zc.zodbutils import TimeStamp for backwards compatibility. I'd rather not do this. Let's be a bit more selective here. The number of imports from ZODB are pretty limited. Many of them should move to transaction. Some of them are just test utilities that can be duplicated. I think the biggest challenge is WeakSet. This could be broken out into a separate package, but I think it's not as general as its name implies and should probably just be moved to transaction. OK. And I'm thinking that the transaction distribution should be named just transaction. Yes, unless we decide to move the package. I think transaction is a bit presumptuous. :) How about zope.transaction? There's a good deal of 3rd-party code that does import transaction but we could supply a module alias for this purpose. We'd just change the Z2 and Z3 appserver distributions to do import zope.transaction as transaction or whatever, and have the appserver distributions depend on a shim transaction module or module alias or whatever too so 3rd-party code would continue to work, maybe making imports using them issue a deprecation warning? - C ___ For more information about ZODB, see the ZODB Wiki: http://www.zope.org/Wikis/ZODB/ ZODB-Dev mailing list - ZODB-Dev@zope.org http://mail.zope.org/mailman/listinfo/zodb-dev
Re: [ZODB-Dev] breaking out the transaction module from ZODB
On Nov 8, 2007, at 1:59 PM, Chris McDonough wrote: On Nov 8, 2007, at 9:14 AM, Jim Fulton wrote: class TransactionFailedError(Exception): Cannot perform an operation on a transaction that previously failed. An attempt was made to commit a transaction, or to join a transaction, but this transaction previously raised an exception during an attempt to commit it. The transaction must be explicitly aborted, either by invoking abort() on the transaction, or begin() on its transaction manager. Why not subclass TransactionError? It didn't before. Should it? Seems logical, but I'm not looking that closely. :) ... And I'm thinking that the transaction distribution should be named just transaction. Yes, unless we decide to move the package. I think transaction is a bit presumptuous. :) How about zope.transaction? Guido recently told me that people in the Python community at large assume that anything in the Zope namespace is assumed to be Zope specific, so I'd rather not put it there. There's a good deal of 3rd-party code that does import transaction Good point. I guess we should leave the package where it is. Note that then we have a tricky issue with avoiding having the package installed twice. I guess we should ignore this for now. :/ but we could supply a module alias for this purpose. We'd just change the Z2 and Z3 appserver distributions to do import zope.transaction as transaction or whatever, and have the appserver distributions depend on a shim transaction module or module alias or whatever too so 3rd-party code would continue to work, maybe making imports using them issue a deprecation warning? Never mind. I guess we should leave it where it is. :) Jim -- Jim Fulton Zope Corporation ___ For more information about ZODB, see the ZODB Wiki: http://www.zope.org/Wikis/ZODB/ ZODB-Dev mailing list - ZODB-Dev@zope.org http://mail.zope.org/mailman/listinfo/zodb-dev
Re: [ZODB-Dev] breaking out the transaction module from ZODB
On Nov 8, 2007, at 6:25 PM, Jim Fulton wrote: Why not subclass TransactionError? It didn't before. Should it? Seems logical, but I'm not looking that closely. :) OK. How about zope.transaction? Guido recently told me that people in the Python community at large assume that anything in the Zope namespace is assumed to be Zope specific, so I'd rather not put it there. Does it matter? People who are allergic to the name zope can probably lose. There's a good deal of 3rd-party code that does import transaction Good point. I guess we should leave the package where it is. Note that then we have a tricky issue with avoiding having the package installed twice. I guess we should ignore this for now. :/ It'd be no problem to provide the shims. - C ___ For more information about ZODB, see the ZODB Wiki: http://www.zope.org/Wikis/ZODB/ ZODB-Dev mailing list - ZODB-Dev@zope.org http://mail.zope.org/mailman/listinfo/zodb-dev
