When adding tag or updating description, lp_save() gives "HTTP Error 412: Precondition Failed"
Affects | Status | Importance | Assigned to | Milestone | |
---|---|---|---|---|---|
| lazr.restful |
High
|
Leonard Richardson | ||
| lazr.restfulclient (Ubuntu) |
High
|
Unassigned | ||
| Lucid |
High
|
Unassigned |
Bug Description
Getting this error consistently when attempting to add a tag using the following code (which worked last time I ran it a few weeks ago).
if not tag in bug.tags:
# Workaround bug #254901
print " ---> Tagged ",tag
I also tested with the commented line instead of the workaround code; same error.
Traceback (most recent call last):
File "./process-
bug.
File "/home/
self.
File "build/
File "build/
File "build/
launchpadlib.
Related branches
- Graham Binns (community): Approve (code) on 2010-03-16
-
Diff: 362 lines (+222/-29)3 files modifiedsrc/lazr/restful/_resource.py (+75/-28)
src/lazr/restful/example/base/tests/entry.txt (+146/-0)
src/lazr/restful/version.txt (+1/-1)
- Gary Poster (community): Approve on 2010-03-17
-
Diff: 246 lines (+140/-26)5 files modifiedlib/canonical/launchpad/pagetests/webservice/apidoc.txt (+1/-0)
lib/canonical/launchpad/pagetests/webservice/conditional-write.txt (+103/-0)
utilities/apidoc-index.pt (+27/-8)
utilities/create-lp-wadl-and-apidoc.py (+8/-17)
versions.cfg (+1/-1)
description: | updated |
Bryce Harrington (bryce) wrote : | #2 |
Leonard Richardson (leonardr) wrote : | #3 |
Bryce Harrington (bryce) wrote : | #4 |
Attached is a minimal script to reproduce the issue.
It seems the issue is not making multiple changes, nor calling lp_save() multiple times. It is sufficient to simply reference a field once before and once after calling lp_save().
Bryce Harrington (bryce) wrote : | #5 |
If I re-get the bug after the lp_save(), it seems to work (attached script is applying both tags properly)
Bryce Harrington (bryce) wrote : | #6 |
Here is a log from the working script in comment #5
Bryce Harrington (bryce) wrote : | #7 |
Ran into the 412 error again with a script that doesn't set any tags, only updates description. Also gets the error after a call to lp_save().
Bryce Harrington (bryce) wrote : | #8 |
This seems to have gone away now.
Martin Pitt (pitti) wrote : | #9 |
I get this problem when trying to set a duplicate:
>>> b.duplicate_of
>>> b.duplicate_of = 341478
>>> b.lp_save()
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "/usr/lib/
URI(
File "/usr/lib/
'PATCH', extra_headers=
File "/usr/lib/
raise HTTPError(response, content)
launchpadlib.
And unlike for comments, it does not actually set the duplicate, so the HTTPError cannot just be ignored.
Changed in launchpadlib: | |
status: | New → Confirmed |
Markus Korn (thekorn) wrote : | #10 |
I cannot confirm this, trying to change the duplicate_of attribute of a bug works for me, I don't get a 'HTTP Error 412'.
Martin, as a side note, giving the bug id in your last example won't work, it has to either
>>> b.duplicate_of = launchpad.
or
>>> b.duplicate_of = "https:/
Markus
Martin Pitt (pitti) wrote : | #11 |
I found a workaround for this now. Previously I was doing
bug = self.launchpad.
for a in bug.attachments:
if a.title in (...):
# MARKER1
if not bug.duplicate_of:
This gave me the afforementioned 412 error. After a looong time of fiddling, I found out the following:
* Deleting attachmends and then changing properties leads to this error in lp_save()
* Calling lp_save() twice in a row leads to this error.
So what I did to circumvent the crash:
* at MARKER1: get a new bug object with "bug = self.launchpad.
* at MARKER2:
if bug._dirty_
I really shouldn't have to explicitly query a private attribute (_dirty_
Markus Korn (thekorn) wrote : | #12 |
I think https:/
Maybe some insider of launchpad's code can mark them as duplicates and rise priority of this bug, for me it looks like a design issue which should be fixed in an early stage of launchpadlib and/or the API
Martin Pitt (pitti) wrote : | #13 |
Argh, this is a nuisance. It also happens in this code:
bug = self.launchpad.
However, the latter was already fixed in an earlier package version than the \
one in this report. This might be a regression or because the problem is \
in a dependent package.' % master,
bug.tags = bug.tags + ['regression-
print bug._dirty_
Changed in launchpadlib: | |
importance: | Undecided → High |
status: | Confirmed → Triaged |
Martin Pitt (pitti) wrote : | #14 |
I just got yet another incarnation of that in the retracers. Good debugging hint from James:
james_w| pitti: and you can "import httplib2;
Leonard Richardson (leonardr) wrote : | #15 |
jkakar reports the same problem running this code: http://
Martin Pitt (pitti) wrote : | #16 |
Did anything change in LP recently in that regard? The retracers crash all over the place since today, while they were grinding happily for over a week before.
I added some debugging code to print the exception's message and content fields, but both are empty. This is from very simple code:
def _mark_dup_
'''Mark crash id as checked for being a duplicate.'''
bug = self.launchpad.
if 'need-duplicate
x = bug.tags[:] # LP#254901 workaround
I. ee. I just create a bug object, change .tags, and save it again. No re-use, or other things.
Do you need more information about this?
Martin Pitt (pitti) wrote : | #17 |
Or is there a workaround for this? This is quite urgent, since crash reports bitrot veeery fast.
Leonard Richardson (leonardr) wrote : | #18 |
I've finally been able to duplicate this. Here's a very edited HTTP dump that shows the problem
===
GET /beta/bugs/353805 HTTP/1.1
if-none-match: "7f7f5e3c50396d
HTTP/1.1 200 Ok
Etag: "3c1d2fa23419ec
---
PATCH /beta/launchpad
if-match: "34ed5fed814bac
HTTP/1.1 412 Precondition Failed
===
The values for If-None-Match and If-Match are totally random. They don't show up anywhere in the cache or anywhere else in the HTTP dump. In fact, even the initial requests to /beta/ have a random If-None-Match.
If I clear the cache, I don't send any If-None-Match headers, but eventually I do send a random If-Match and I get the same error.
Leonard Richardson (leonardr) wrote : | #19 |
OK, I found at least one problem that causes 412 errors. When you PATCH a representation you get back 209 ("Content Returned") with a new representation. The http_etag in this representation is the random ETag that's sent on a subsequent PATCH and then you get 412.
---
PATCH /+bug/353805: Sending If-Match with http_etag "84bb970299f2ae
209 Content Returned: New representation has http_etag "53a61af57b930f
---
PATCH /+bug/353805: Sending If-Match with http_etag "53a61af57b930f
412 Precondition Failed
---
The http_etag here is probably generated from some sort of transitional state that's neither the original state nor the final state. I'm looking into it now.
Leonard Richardson (leonardr) wrote : | #20 |
This is a problem specific to Launchpad bugs. The problem does not exist in the lazr.restful example web service, and it doesn't exist for other Launchpad resources (such as people). Everyone who's commented on this thread has had a problem with bugs.
The culprit is the date_last_updated field. Here are the only differences between the version of the bug received along with a 209, and the same bug retrieved an instant later with a fresh GET request.
date_last_updated: 2006-07-
http_etag: "490b68f89bc09b
date_last_updated is updated after the ETag is calculated, possibly in response to the ObjectModifiedE
Leonard Richardson (leonardr) wrote : | #21 |
The culprit is a combination of the unmarshalled_
I have a fix but since the lazr.restful example service doesn't use OMEs at all, a proper test will take a little while.
Bryce Harrington (bryce) wrote : Re: [Bug 336866] Re: When adding tag or updating description, lp_save() gives "HTTP Error 412: Precondition Failed" | #22 |
Awesome! Looking forward to trying it out.
On Thu, Apr 23, 2009 at 05:53:03PM -0000, Leonard Richardson wrote:
> The culprit is a combination of the unmarshalled_
> ObjectModifiedE
> lazr.restful didn't hear about it, and only removed the fields the
> client modified from unmarshalled_
> date_last_updated stays in there.
>
> I have a fix but since the lazr.restful example service doesn't use OMEs
> at all, a proper test will take a little while.
>
> --
> When adding tag or updating description, lp_save() gives "HTTP Error 412: Precondition Failed"
> https:/
> You received this bug notification because you are a direct subscriber
> of the bug.
>
> Status in Launchpad web services client library: Triaged
>
> Bug description:
> Getting this error consistently when attempting to add a tag using the following code (which worked last time I ran it a few weeks ago).
>
> if not tag in bug.tags:
> #bug.tags.
> # Workaround bug #254901
> tag_list = bug.tags
> tag_list.
> bug.tags = tag_list
>
> print " ---> Tagged ",tag
> bug.lp_save()
>
> I also tested with the commented line instead of the workaround code; same error.
>
> Traceback (most recent call last):
> File "./process-
> bug.append_
> File "/home/
> self.bug.lp_save()
> File "build/
> File "build/
> File "build/
> launchpadlib.
affects: | launchpadlib → lazr.restful |
Changed in lazr.restful: | |
assignee: | nobody → Leonard Richardson (leonardr) |
James Westby (james-w) wrote : | #23 |
Adding a python-launchpadlib task in Ubuntu, as I will want to port your
changes to the Jaunty package.
I'll package the new python-launchpadlib for Karmic with lazr-restfulclient
and any other new packages that will be needed, but doing that for Jaunty
is not really possible.
Thanks,
James
Leonard Richardson (leonardr) wrote : | #24 |
This is a server-side bug, so there's nothing to do for launchpadlib.
I've gotten my branch reviewed and landed in the main lazr.restful branch. Because Launchpad doesn't use that branch yet I'm going to be backporting it.
James Westby (james-w) wrote : | #25 |
Ah, sorry, I got confused between lazr.restful and lazr.restfulclient.
Thanks for fixing this, I'm sure that there will be a lot less hair torn out
by some of my fellow developers now.
Thanks,
James
Changed in python-launchpadlib (Ubuntu): | |
status: | New → Invalid |
Leonard Richardson (leonardr) wrote : | #26 |
I've backported the fix to the lazr.restful branch used by Launchpad.
Changed in lazr.restful: | |
status: | Triaged → Fix Committed |
Martin Pitt (pitti) wrote : | #27 |
I still get daily crashes from the Apport retracer in this simple code:
bug = self.launchpad.
if self.arch_tag in bug.tags:
x = bug.tags[:] # LP#254901 workaround
File "/home/
URI(
File "/home/
'PATCH', extra_headers=
File "/home/
raise HTTPError(response, content)
launchpadlib.
So it doesn't actually seem fixed yet?
James Westby (james-w) wrote : | #28 |
I've just hit Precondition Failed as well, just with
branch.
branch.
though maybe it's a case of different bug, same manifestation.
Any advice would be appreciated, as I know of no other way to achieve this.
Thanks,
James
James Westby (james-w) wrote : | #29 |
Hi,
I may have spoken too soon. I can't reproduce the error in testing, so
I've added some code to give more information if this happens again and
will wait and see what happens.
Thanks,
James
Changed in lazr.restful: | |
status: | Fix Committed → Fix Released |
tags: | added: tech-debt |
Martin Pitt (pitti) wrote : | #30 |
Reopening. When I remove the workarounds for this bug in Apport's launchpad module, I still get the same problem:
File "apport/
bug.lp_save()
File "/usr/lib/
URI(
File "/usr/lib/
'PATCH', extra_headers=
File "/usr/lib/
raise HTTPError(response, content)
HTTPError: HTTP Error 412: Precondition Failed
Response headers:
---
content-length: 0
content-type: text/plain
date: Wed, 20 Jan 2010 11:48:55 GMT
server: zope.server.http (HTTP)
status: 412
vary: Cookie,
via: 1.1 wildcard.
x-content-
x-powered-by: Zope (www.zope.org), Python (www.python.org)
---
Response body:
---
---
Changed in lazr.restful: | |
status: | Fix Released → Confirmed |
Martin Pool (mbp) wrote : | #31 |
This is hitting me too in Hydrazine, and it's fairly easily reproducible there if you make changes quickly. It does seems timing-dependent which would be consistent with it relating to a last-modified field stored on the server: it seems like if you get two updates within a 1s window then it crashes.
Martin Pitt (pitti) wrote : | #32 |
Apport already has workarounds for this, but apparently still not enough. This still keeps people from submitting debug data to Launchpad, see bug 528680.
Can the priority of this please be bumped?
affects: | python-launchpadlib (Ubuntu) → lazr.restfulclient (Ubuntu) |
Changed in lazr.restfulclient (Ubuntu): | |
importance: | Undecided → High |
status: | Invalid → Triaged |
James Westby (james-w) wrote : Re: [Bug 336866] Re: When adding tag or updating description, lp_save() gives "HTTP Error 412: Precondition Failed" | #33 |
On Tue, 02 Mar 2010 11:11:47 -0000, Martin Pitt <email address hidden> wrote:
> Apport already has workarounds for this, but apparently still not
> enough. This still keeps people from submitting debug data to Launchpad,
> see bug 528680.
>
> Can the priority of this please be bumped?
Martin, how sure are you that these are spurious?
Is there a chance that they are actually legitimate errors due to
something else modifying the bug between you fetching it and saving it?
Thanks,
James
Martin Pitt (pitti) wrote : | #34 |
The window for that is pretty small, since it's an automated script with not much in between. Martin-Eric said that he tried this a number of times, and other people saw this as well.
Also, it's perfectly reproducible in the Apport test suite if I remove all the workarounds for this bug.
James Westby (james-w) wrote : | #35 |
On Tue, 02 Mar 2010 12:39:09 -0000, Martin Pitt <email address hidden> wrote:
> The window for that is pretty small, since it's an automated script with
> not much in between. Martin-Eric said that he tried this a number of
> times, and other people saw this as well.
>
> Also, it's perfectly reproducible in the Apport test suite if I remove
> all the workarounds for this bug.
Ok, thanks. I don't do either of these things in scripts myself, so
haven't seen it.
I did however do things with branches over the API that was getting 412
a lot, but this came down to automated tasks that LP does in the
background the modify the branch happening in about the same amount of
time after pushing the branch that my script took to get to the API
stage, leading to legitimate 412 errors that were easy to work around
once I understood what was going on.
Thanks,
James
James Westby (james-w) wrote : | #36 |
Right, I can still reproduce by doing
bug = lp.bugs[12345]
bug.newMessag
bug.tags = ["foo"]
bug.lp_save()
because date_last_updated and date_last_message change when we call the
newMessage method, but we don't update the representation.
The original fix for this bug was server side. Leonard, is there something we can
change server side for this?
On the client side, we could fix this problem by changing NamedOpearation
which currently does
if response.status == 201:
return self._handle_
else:
if http_method == 'post':
# The method call probably modified this resource in
# an unknown way. Refresh its representation.
return self._handle_
where we could move the the if http_method block outside the other if block, so that
we refresh when we create the new message.
What's the best approach?
Thanks,
James
Leonard Richardson (leonardr) wrote : | #37 |
If creating a new resource secretly modifies the parent resource, then the client-side fix is the correct thing to do. But I don't know if that would help the apport people. Can someone show me the apport code that has the problem, and the workarounds devised for it?
Martin Pool (mbp) wrote : | #38 |
https:/
James Westby (james-w) wrote : | #39 |
I believe all the API using code for apport can be found in
It all looks pretty straightforward to me.
You could grab lp:apport and try running the testsuite, and find and remove the
workarounds if it doesn't show the failures.
Thanks,
James
James Westby (james-w) wrote : | #40 |
So, Martin helpfully labelled all the places where workarounds were added,
we have them after
a.removeFromBug()
where a is an attachment
bug.newMessage
and changing bug.tags, which is one that I can't reproduce.
Thanks,
James
Leonard Richardson (leonardr) wrote : | #41 |
I know of two new bug fields that change behind-the-scenes automatically, bug_heat and task_age. task_age is going away, so let's ignore that. I'd like to fix bug_heat and then see if there's a separate problem to do with named operations.
Here's a paste of a message I posted to launchpad-dev about solving the bug_heat problem. I'm still working on this.
1. If a field is important enough to include in the representation, it's
important enough to include in the ETag. As Robert points out, omitting
a read-only field from the ETag will stop clients from knowing when the
value changes.
2. I like Bjorn's idea of a two-part ETag, where both parts are checked
for GET requests/caching, but only one part is checked for PATCH/PUT
requests. But, I don't think this would comply with the HTTP standard.
There are two types of ETags, "strong" and "weak". A strong ETag changes
whenever "the entity (the entity-body or any entity-headers) changes in
any way". A weak ETag changes "only on semantically significant changes,
and not when insignificant aspects of the entity change". These quotes
are from section 13.3.3.
Consider a two-part ETag, where one part describes the state of the
read-only fields and the other part describes the state of the
read-write fields. Taken together, the whole thing is a strong ETag.
Look at only the second part and you have a weak ETag.
But (13.3.3 again) "The only function that the HTTP/1.1 protocol defines
on [ETags] is comparison." There's no looking at the second half of an
ETag. The whole thing has to match.
OK, so let's define another function on ETags, another "weak comparison
function" which only looks at the second half of the ETag. That goes
beyond HTTP/1.1 but it doesn't contradict the standard. Now:
We would like to validate GET requests using the strong comparison
function (looking at the strong ETag), so that even minor changes will
invalidate the cache and the client will always have the most up-to-date
information. We would like to validate PATCH requests by using the weak
comparison function, so that irrelevant changes to the resource don't
affect the success of the PATCH.
But, the HTTP standard says we have to do the opposite. Section 14.26
says "The weak comparison function can only be used with GET or HEAD
requests."
The IETF draft that defines PATCH says, "Clients wishing to apply a
patch document to a known entity can first acquire the strong ETag of
the resource to be modified, and use that Etag in the If-Match header on
the PATCH request to verify that the resource is still unchanged."
As I read the standards, if bug_heat has changed and I try to PATCH an
unrelated field, the correct behavior is that my PATCH should fail. I
don't think this makes sense for us given the way we implement PATCH,
but it does make sense in general.
I don't think anything bad will happen if we implement the two-part
ETag. The whole ETag string is in fact a strong ETag, so anything that
treats it as a strong ETag will still work. We'll just have some
server-side magic that secretly implements a weak comparison function to
make PATCH requests work. And if anyone actually tries to PATCH bug_heat
they'll still get an error...
James Westby (james-w) wrote : | #42 |
On Tue, 09 Mar 2010 18:55:48 -0000, Leonard Richardson <email address hidden> wrote:
> I know of two new bug fields that change behind-the-scenes
> automatically, bug_heat and task_age. task_age is going away, so let's
> ignore that. I'd like to fix bug_heat and then see if there's a separate
> problem to do with named operations.
There is a separate problem related to named operations, on bugs at
least, due to the dates of operations that are exported.
They are however read-only and so your proposal would allow them to
function assuming that those dates were not in the strong etag.
The lack of refresh means that the local representation will be out of
date after some named operations though.
Thanks,
James
Martin Pitt (pitti) wrote : | #43 |
There's a multitude of ways to reproduce this still. One is to add a comment and then set the privacy flag, see attached reproducer script.
$ python bug336866.py
Traceback (most recent call last):
File "bug336866.py", line 9, in <module>
b.lp_save()
File "/usr/lib/
URI(
File "/usr/lib/
'PATCH', extra_headers=
File "/usr/lib/
raise HTTPError(response, content)
lazr.restfulcli
Response headers:
---
content-length: 0
content-type: text/plain
date: Fri, 12 Mar 2010 11:33:07 GMT
server: zope.server.http (HTTP)
status: 412
vary: Cookie,
via: 1.1 wildcard.
x-content-
x-powered-by: Zope (www.zope.org), Python (www.python.org)
---
Response body:
---
---
James Westby (james-w) wrote : | #44 |
On Fri, 12 Mar 2010 11:34:23 -0000, Martin Pitt <email address hidden> wrote:
> There's a multitude of ways to reproduce this still. One is to add a
> comment and then set the privacy flag, see attached reproducer script.
>
> $ python bug336866.py
> Traceback (most recent call last):
> File "bug336866.py", line 9, in <module>
> b.lp_save()
> File "/usr/lib/
> URI(self.
> File "/usr/lib/
> 'PATCH', extra_headers=
> File "/usr/lib/
> raise HTTPError(response, content)
> lazr.restfulcli
Yep, that's the NamedOperation case again.
Thanks,
James
Leonard Richardson (leonardr) wrote : | #45 |
I think the linked branch will resolve this problem. The only exception is if there is a named operation that modifies a field you can also modify directly, like IBug.status. Of course, this is what mutator methods like transitionToStatus do, but we got rid of those methods in version 1.0 of the web service, partly for this reason.
Are there any named operations that modify a field you can modify directly, but that aren't mutator methods whose _only_ purpose is to modify that field? Ie. operations that will still exist in version 1.0 of the web service.
James Westby (james-w) wrote : | #46 |
On Tue, 16 Mar 2010 12:31:48 -0000, Leonard Richardson <email address hidden> wrote:
> I think the linked branch will resolve this problem. The only exception
> is if there is a named operation that modifies a field you can also
> modify directly, like IBug.status. Of course, this is what mutator
> methods like transitionToStatus do, but we got rid of those methods in
> version 1.0 of the web service, partly for this reason.
>
> Are there any named operations that modify a field you can modify
> directly, but that aren't mutator methods whose _only_ purpose is to
> modify that field? Ie. operations that will still exist in version 1.0
> of the web service.
IBug.setPrivate and IBug.markAsDupl
https:/
does that indicate they should be killed too?
(Plus, I notice that isUserAffected is exported as a POST which
indicates a bug somewhere).
Thanks,
James
Leonard Richardson (leonardr) wrote : | #47 |
Yes, I think setPrivate can be made the mutator for 'private' and markAsDuplicate the mutator for 'duplicate_
Fixed in stable r10544 <http://
Changed in lazr.restful: | |
status: | Confirmed → Fix Committed |
tags: | added: qa-needstesting |
James Westby (james-w) wrote : | #49 |
This appears to work ok.
Thanks,
James
tags: |
added: qa-ok removed: qa-needstesting |
Changed in lazr.restfulclient (Ubuntu Lucid): | |
status: | Triaged → Invalid |
James Westby (james-w) wrote : | #50 |
Closing the lazr.restfulclient task as this was a server-side fix.
Thanks,
James
Martin Pitt (pitti) wrote : | #51 |
This is still happening with current Launchpad and precise's launchpadlib. It happens when I try to modify bug.title.
Robert Collins (lifeless) wrote : | #52 |
@pitti please file a new bug for that
Martin Pitt (pitti) wrote : | #53 |
Seems I cannot reproduce it right now with setting .title, so perhaps that got fixed in latest 1.9.12 launchpadlib. Thanks!
Changed in lazr.restful: | |
status: | Fix Committed → Fix Released |
Here are the last two requests:
send: 'PATCH /beta/bugs/292317 3cfeb7e05896b38 208e23ddc75
if-match: "72a9365dbdc47f
send: '{"tags": ["needs-xorglog"]}'
reply: 'HTTP/1.1 209 Content Returned\r\n'
send: 'PATCH /beta/bugs/292317 a42d5d0e5bc4aaf 14be1e73123" lspci-vvnn" ]}'
if-match: "7f60495c065527
send: '{"tags": ["needs-xorglog", "needs-
reply: 'HTTP/1.1 412 Precondition Failed\r\n'
Are you calling lp_save() twice, or just once? It would be helpful if you made a GET request after every call to lp_save(), to see what the server thinks the ETag is at each stage.
This might be a problem with the way launchpadlib handles the 209 response code.