Re: [Checkins] SVN: zope.server/trunk/s fix of 599 error on conflict error in request
Hi, Adam Groszer schrieb:
Log message for revision 83382: fix of 599 error on conflict error in request see: http://mail.zope.org/pipermail/zope-dev/2008-January/030844.html
Changed: U zope.server/trunk/setup.py U zope.server/trunk/src/zope/server/ftp/publisher.py
-=- Modified: zope.server/trunk/setup.py =================================================================== --- zope.server/trunk/setup.py 2008-02-01 15:08:00 UTC (rev 83381) +++ zope.server/trunk/setup.py 2008-02-01 15:19:32 UTC (rev 83382) @@ -30,10 +30,10 @@ long_description=open('README.txt').read(),
packages=find_packages('src'), - package_dir = {'': 'src'}, + package_dir = {'': 'src'},
namespace_packages=['zope',], - + tests_require = ['zope.testing', 'zope.i18n', 'zope.component'], @@ -41,7 +41,8 @@ 'zope.interface', 'zope.publisher', 'zope.security', - 'zope.deprecation'], + 'zope.deprecation', + 'ZODB3'], include_package_data = True, zip_safe = False, entry_points = """
Please try to avoid mixing mixing checkins with different purposes. This whitespace change should be a separate checkin.
Modified: zope.server/trunk/src/zope/server/ftp/publisher.py =================================================================== --- zope.server/trunk/src/zope/server/ftp/publisher.py 2008-02-01 15:08:00 UTC (rev 83381) +++ zope.server/trunk/src/zope/server/ftp/publisher.py 2008-02-01 15:19:32 UTC (rev 83382) @@ -51,7 +51,7 @@ return self._execute(path, 'ls', split=False, filter=filter)
def readfile(self, path, outstream, start=0, end=None): - return self._execute(path, 'readfile', + return self._execute(path, 'readfile', outstream=outstream, start=start, end=end)
def lsinfo(self, path): @@ -108,9 +108,12 @@
# Note that publish() calls close() on request, which deletes the # response from the request, so that we need to keep track of it. - response = request.response - publish(request) - return response.getResult() + # agroszer: 2008.feb.1.: currently the above seems not to be true + # request will KEEP the response on close() + # even more if a retry occurs in the publisher, + # the response will be LOST, so we must accept the returned request + request = publish(request) + return request.response.getResult()
Same comment as previously, please avoid this style of annotation. Also, it doesn't look like the issue is actually finally resolved as you say `seems`. SVN tracks who edited what and when, the statement of your name and the change date isn't necessary. Again, a test case is missing. Please review your other related checkins as well. Christian -- gocept gmbh & co. kg - forsterstrasse 29 - 06112 halle (saale) - germany www.gocept.com - ct@gocept.com - phone +49 345 122 9889 7 - fax +49 345 122 9889 1 - zope and plone consulting and development
Hello Christian, I'm sorry Christian. Points taken, I'll try to do better next time. Actually the issue was an edge case. See my mail: http://mail.zope.org/pipermail/zope-dev/2008-January/030844.html Solution is to change: response = request.response publish(request) #do something with response to: request = publish(request) response = request.response #(or equivalent) The missing tests are missing, because it seemed not that easy to do it. I would appreciate some help. btw, all existing tests pass. Monday, February 4, 2008, 1:23:24 PM, you wrote: CT> Hi, CT> Adam Groszer schrieb:
Log message for revision 83382: fix of 599 error on conflict error in request see: http://mail.zope.org/pipermail/zope-dev/2008-January/030844.html
Changed: U zope.server/trunk/setup.py U zope.server/trunk/src/zope/server/ftp/publisher.py
-=- Modified: zope.server/trunk/setup.py =================================================================== --- zope.server/trunk/setup.py 2008-02-01 15:08:00 UTC (rev 83381) +++ zope.server/trunk/setup.py 2008-02-01 15:19:32 UTC (rev 83382) @@ -30,10 +30,10 @@ long_description=open('README.txt').read(),
packages=find_packages('src'), - package_dir = {'': 'src'}, + package_dir = {'': 'src'},
namespace_packages=['zope',], - + tests_require = ['zope.testing', 'zope.i18n', 'zope.component'], @@ -41,7 +41,8 @@ 'zope.interface', 'zope.publisher', 'zope.security', - 'zope.deprecation'], + 'zope.deprecation', + 'ZODB3'], include_package_data = True, zip_safe = False, entry_points = """
CT> Please try to avoid mixing mixing checkins with different purposes. This CT> whitespace change should be a separate checkin.
Modified: zope.server/trunk/src/zope/server/ftp/publisher.py =================================================================== --- zope.server/trunk/src/zope/server/ftp/publisher.py 2008-02-01 15:08:00 UTC (rev 83381) +++ zope.server/trunk/src/zope/server/ftp/publisher.py 2008-02-01 15:19:32 UTC (rev 83382) @@ -51,7 +51,7 @@ return self._execute(path, 'ls', split=False, filter=filter)
def readfile(self, path, outstream, start=0, end=None): - return self._execute(path, 'readfile', + return self._execute(path, 'readfile', outstream=outstream, start=start, end=end)
def lsinfo(self, path): @@ -108,9 +108,12 @@
# Note that publish() calls close() on request, which deletes the # response from the request, so that we need to keep track of it. - response = request.response - publish(request) - return response.getResult() + # agroszer: 2008.feb.1.: currently the above seems not to be true + # request will KEEP the response on close() + # even more if a retry occurs in the publisher, + # the response will be LOST, so we must accept the returned request + request = publish(request) + return request.response.getResult()
CT> Same comment as previously, please avoid this style of annotation. Also, CT> it doesn't look like the issue is actually finally resolved as you say CT> `seems`. CT> SVN tracks who edited what and when, the statement of your name and the CT> change date isn't necessary. CT> Again, a test case is missing. CT> Please review your other related checkins as well. CT> Christian -- Best regards, Adam Groszer mailto:agroszer@gmail.com -- Quote of the day: The attacker must vanquish; the defender need only survive.
Christian Theune wrote at 2008-2-4 13:23 +0100:
... + # agroszer: 2008.feb.1.: currently the above seems not to be true + # request will KEEP the response on close() + # even more if a retry occurs in the publisher, + # the response will be LOST, so we must accept the returned request + request = publish(request) + return request.response.getResult() Same comment as previously, please avoid this style of annotation.
I disagree with you. Commenting difficult code passages which can easily be seen as overly complex if one does not look carefully is a *very* good idea. Especially in this case, I find the explanation vital why "response" is recomputed rather than the already known "response" used.
Also, it doesn't look like the issue is actually finally resolved as you say `seems`.
Better specify explicitely when one is not absolute sure.
SVN tracks who edited what and when, the statement of your name and the change date isn't necessary.
But when one looks at the code, it is not easy to find out which SVN revision produced this code -- unless you look through the complete history. Therefore, the who may be helpful in case of questions about the code. -- Dieter
On Tue, Feb 05, 2008 at 07:41:13PM +0100, Dieter Maurer wrote:
SVN tracks who edited what and when, the statement of your name and the change date isn't necessary.
But when one looks at the code, it is not easy to find out which SVN revision produced this code -- unless you look through the complete history.
It is somewhat easier if your editor properly supports 'svn annotate'. On the other hand, names in comments, unlike svn's knowledge, survive when you move packages around.
Therefore, the who may be helpful in case of questions about the code.
Marius Gedminas -- Prediction is very difficult, especially of the future. -- Niels Bohr
Hi, Dieter Maurer schrieb:
Christian Theune wrote at 2008-2-4 13:23 +0100:
... + # agroszer: 2008.feb.1.: currently the above seems not to be true + # request will KEEP the response on close() + # even more if a retry occurs in the publisher, + # the response will be LOST, so we must accept the returned request + request = publish(request) + return request.response.getResult() Same comment as previously, please avoid this style of annotation.
I disagree with you.
Commenting difficult code passages which can easily be seen as overly complex if one does not look carefully is a *very* good idea.
Especially in this case, I find the explanation vital why "response" is recomputed rather than the already known "response" used.
I haven't been clear enough about what I meant with `style of annotation`. IMHO this issue should have a bug number. The bug number should be annotated instead of a name and a date, stating the issue in short and making it possible to lookup discussion and proceeding of this bug in the tracker.
Also, it doesn't look like the issue is actually finally resolved as you say `seems`.
Better specify explicitely when one is not absolute sure.
That's what bugtracking is for.
SVN tracks who edited what and when, the statement of your name and the change date isn't necessary.
But when one looks at the code, it is not easy to find out which SVN revision produced this code -- unless you look through the complete history.
svn (praise|blame) tells you by whom and in which revision a line of code was changed last time. Christian -- gocept gmbh & co. kg - forsterstrasse 29 - 06112 halle (saale) - germany www.gocept.com - ct@gocept.com - phone +49 345 122 9889 7 - fax +49 345 122 9889 1 - zope and plone consulting and development
Dieter Maurer wrote:
SVN tracks who edited what and when, the statement of your name and the change date isn't necessary.
But when one looks at the code, it is not easy to find out which SVN revision produced this code -- unless you look through the complete history.
I think 'svn annotate' will give this detail. Regards, Baiju M
participants (5)
-
Adam Groszer -
Baiju M -
Christian Theune -
Dieter Maurer -
Marius Gedminas