changing bugtask attributes via api gives "412 precondition failed"

Martin,

Could you please dump the json of the task before you get the error, and then after a .lp_refresh()
after you get it. This will show whether something has changed, or there is an etag calculation issue.

This is a problem which there can be many causes for, and so we need to no which it is
before we can look at tackling it.

You can dump the json with something like

  print bug_task._wadl_resource._definition.representation

Doing bug_task.lp_refresh() will then fetch it again from the server and so printing the same
thing will show what changed, if anything.

Thanks,

James

I changed bugclient like so:

    def do_importance(self, line):
        """Set importance"""
        task = self._needs_single_task()
        if task is None:
            return
        new_importance = canonical_importance(line)
        if new_importance is None:
            return
        print 'changing importance %s => %s' % (task.importance, new_importance)
        task.importance = new_importance
        print '**** before save'
        if opts.debug:
            print task._wadl_resource._definition.representation
        try:
            task.lp_save()
        except:
            print '**** got error'
            if opts.debug:
                print task._wadl_resource._definition.representation

The before form is

{u'date_closed': None, u'task_age': 53680, u'date_assigned': None, u'title': u'Bug #532600 in Bazaar: "bzr diff --summarize"', u'bug_link': u'https://api.edge.launchpad.net/beta/bugs/532600', u'bug_watch_link': None, u'milestone_link': None, u'http_etag': u'"93faa3919aaa3a58289ace9b3cfa8d035f8fd49a"', u'date_left_closed': u'2010-03-05T22:45:34.103676+00:00', u'date_fix_committed': None, u'date_fix_released': None, u'self_link': u'https://api.edge.launchpad.net/beta/bzr/+bug/532600', u'resource_type_link': u'https://api.edge.launchpad.net/beta/#bug_task', u'status': u'Confirmed', u'bug_target_name': u'bzr', u'importance': u'Low', u'assignee_link': None, u'date_triaged': None, u'date_in_progress': None, u'target_link': u'https://api.edge.launchpad.net/beta/bzr', u'bug_target_display_name': u'Bazaar', u'related_tasks_collection_link': u'https://api.edge.launchpad.net/beta/bzr/+bug/532600/related_tasks', u'date_confirmed': u'2010-03-08T00:07:26.022635+00:00', u'date_left_new': u'2010-03-05T15:00:45.944280+00:00', u'owner_link': u'https://api.edge.launchpad.net/beta/~pelle-morth', u'date_created': u'2010-03-05T12:28:14.302618+00:00', u'is_complete': False}

the after form is

{u'date_closed': None, u'task_age': 53680, u'date_assigned': None, u'title': u'Bug #532600 in Bazaar: "bzr diff --summarize"', u'bug_link': u'https://api.edge.launchpad.net/beta/bugs/532600', u'bug_watch_link': None, u'milestone_link': None, u'http_etag': u'"93faa3919aaa3a58289ace9b3cfa8d035f8fd49a"', u'date_left_closed': u'2010-03-05T22:45:34.103676+00:00', u'date_fix_committed': None, u'date_fix_released': None, u'self_link': u'https://api.edge.launchpad.net/beta/bzr/+bug/532600', u'resource_type_link': u'https://api.edge.launchpad.net/beta/#bug_task', u'status': u'Confirmed', u'bug_target_name': u'bzr', u'importance': u'Low', u'assignee_link': None, u'date_triaged': None, u'date_in_progress': None, u'target_link': u'https://api.edge.launchpad.net/beta/bzr', u'bug_target_display_name': u'Bazaar', u'related_tasks_collection_link': u'https://api.edge.launchpad.net/beta/bzr/+bug/532600/related_tasks', u'date_confirmed': u'2010-03-08T00:07:26.022635+00:00', u'date_left_new': u'2010-03-05T15:00:45.944280+00:00', u'owner_link': u'https://api.edge.launchpad.net/beta/~pelle-morth', u'date_created': u'2010-03-05T12:28:14.302618+00:00', u'is_complete': False}

there doesn't seem to be any difference between them.

On Mon, 08 Mar 2010 03:24:23 -0000, Martin Pool <email address hidden> wrote:
> there doesn't seem to be any difference between them.

OK, please ask a LOSA to check if all the edge appservers are on the
same revision currently, that's one thing that can break things in this
manner.

Thanks,

James

<mthaddon> poolie: they are, yes (the same as lpnet/production)

fwiw the same problem occurs when trying to retarget the task to a different product.

I can confirm this bug, all PATCH operations on bug_task objects fail in this way. As a workaround you can use bug_task.transitionTo* which is using POST and works fine.

Note that this workaround will not be working for long. AFAIK we're now in the process of modifying the API to block calls to mutator methods if the attribute they modify is exposed.

task_age is back in the representation, that's why you can't currently modify
anything with PATCH.

Thanks,

James

Wow, that's a surprise. I wonder how that happened. Maybe it slipped through some conflict resolution? Anyway, the fix is easy.

Ok, there's clearly a problem with the ETag generation. Fixing it will not fill fix all our 412 errors (eg. bug 336866), but it's a start.

task_age changes every second, which means the generated ETag changes every second, which means all PATCH requests will result in 412 errors. Clearly changes to task_age should not affect the generated ETag.

HOWEVER, by the same token, I suggest that task_age is useless and should not be published. If you are using launchpadlib you have no way of knowing how old your representation of a bugtask is. Maybe it's an hour old, maybe it's a day old. What's the right value for task_age? You don't know, and there's no way to find out. Changes to task_age don't affect the ETag, so you'll never be able to convince launchpadlib that it has an out-of-date representation. So long as nothing but task_age has changed, Launchpad will tell your client that it already has the most up-to-date representation.

task_age isn't even necessary, since you can subtract date_created from the current time. If I had to guessed I'd say it was published to make it easier to write some Ajax code, but it's causing more problems than it's worth, and I'm not confident that an Ajax application won't have the same problem with out-of-date values as launchpadlib. I suggest we get rid of the field.

That said, it's likely there are more fields that are screwing up the ETags in this way, and not all of them can simply be removed. What are the other candidates? And do you agree about getting rid of task_age?

Just to clarify, the immediate solution to this bug is the removal of task_age. This was already done last cycle by James but the code got reverted by mistake. See the branches attached.

Sorry I didn't make this clearer when I triaged this critical before. It's marked critical to help us ensure we get this branch of James' landed again, since a bad merge caused this to be removed, as Tom noted.

As for your other question, Leonard, I can't think of any other fields that are likely to cause this much of a problem.

Cheers,
deryck

Fixed in stable r10464 <http://bazaar.launchpad.net/~launchpad-pqm/launchpad/stable/revision/10464>

Can someone ack here that launchpadlib scripts are fixed now? James W. maybe?

Cheers,
deryck

On 10 March 2010 16:03, Deryck Hodge <email address hidden> wrote:
> Can someone ack here that launchpadlib scripts are fixed now?  James W.
> maybe?

Note that you can only test this on edge or staging for now. The fix
has not yet been rolled onto the main production servers.

On 11 March 2010 03:38, Tom Berger <email address hidden> wrote:
> On 10 March 2010 16:03, Deryck Hodge <email address hidden> wrote:
>> Can someone ack here that launchpadlib scripts are fixed now?  James W.
>> maybe?
>
> Note that you can only test this on edge or staging for now. The fix
> has not yet been rolled onto the main production servers.

nack, testing against staging using hydrazine bugclient shows the same
failure still occurs.

Just branch hydrazine and run

./bugclient --debug --staging -c 'pillar bzr' -c select_new -c 'triage
confirmed'

--
Martin <http://launchpad.net/~mbp/>

edge however is ok.

python -c 'from launchpadlib.launchpad import Launchpad; lp = Launchpad.login_with("testing", service_root="edge"); task = lp.bugs[12345].bug_tasks[0]; print hasattr(task, "task_age")'

should print False if all is ok.

I would hope that all LP developers were familiar enough with using the API that
you could knock up one-liners such as this yourselves :-)

Thanks,

James

Yes, working for me on edge now too.

--
Martin <http://launchpad.net/~mbp/>

Just a question: if the fix was committed but the bug was returned to In Progress, it should be marked as qa-bad, right?

On 11 March 2010 15:45, Ursula Junque <email address hidden> wrote:
> Just a question: if the fix was committed but the bug was returned to In
> Progress, it should be marked as qa-bad, right?

I don't know. I just thought I'd reopen it because it didn't work in
the way Tom said it would.

--
Martin <http://launchpad.net/~mbp/>

Thanks for the confirmation, Martin and James.

This was part QA and part follow up, which is why I didn't write the one-liner myself, James. ;)

Fixed released in launchpad-project 10.03.