Merge lp://staging/~barry/ubuntuone-storage-protocol/lp1077092 into lp://staging/ubuntuone-storage-protocol

Thanks for working on this!

One comment: the oauth_authenticate method changed changed here is only used (afaict) from lp:ubuntuone-client ActionQueue.authenticate(), and with a different signature. That means that with this change a new branch for ubuntuone-client would be needed.

Instead, I propose that the changes to the signature of oauth_authenticate in this branch are reverted, and that the compatibility properties defined in lp:~barry/ubuntuone-client/lp1077089 are used instead, like:

135 + client = Client(token.key, token.secret, consumer.key, consumer.secret ...

Does this seem reasonable?

On Feb 06, 2013, at 02:04 PM, Alejandro J. Cura wrote:

>One comment: the oauth_authenticate method changed changed here is only used
>(afaict) from lp:ubuntuone-client ActionQueue.authenticate(), and with a
>different signature. That means that with this change a new branch for
>ubuntuone-client would be needed.

Ah right, my changes to ubuntuone-client didn't change the call signature over
there.

>Instead, I propose that the changes to the signature of oauth_authenticate in
>this branch are reverted, and that the compatibility properties defined in
>lp:~barry/ubuntuone-client/lp1077089 are used instead, like:
>
>135 + client = Client(token.key, token.secret, consumer.key, consumer.secret ...
>
>Does this seem reasonable?

It does, good catch.

I suppose if we wanted to get rid of this API backward compatibility hack, we
could do that in a future branch.

Branch update.

I'm trying to run both branches together like this:
(make sure to ./run-tests in the u1sp branch first, so protobuf files get generated).

alecu@bollo:~/canonical/ubuntuone-client/review_lp1077089$ u1sdtool -q; PYTHONPATH=.:~/canonical/ubuntuone-storage-protocol/review_lp1077092/ ./bin/ubuntuone-syncdaemon --debug

The syncdaemon debug log shows that the authentication fails when using the new branches, and I'm able to authenticate using the system's syncdaemon, so there must be something that's working differently.

Phew! That was a fun one to track down and fix. ;)

Let's start by disabling the dbus auto-restart of the ubuntuone-syncdaemon on
a production system. You have to do this or the `u1sdtool -q` command is
ineffectual. It does quit the daemon, but dbus re-activates it so the one in
bin/ubuntuone-syncdaemon will never be able to start.

The easiest way I found for doing this was to comment out the Name and Exec
lines in /usr/share/dbus-1/services/com.ubuntuone.SyncDaemon.service, then
kill -HUP dbus to re-read its service files. Now when you `u1sdtool -q`, it
kills the running syncdaemon and dbus won't restart it.

On Feb 07, 2013, at 12:13 AM, Alejandro J. Cura wrote:

>I'm trying to run both branches together like this:
>(make sure to ./run-tests in the u1sp branch first, so protobuf files get generated).
>
>alecu@bollo:~/canonical/ubuntuone-client/review_lp1077089$ u1sdtool -q; PYTHONPATH=.:~/canonical/ubuntuone-storage-protocol/review_lp1077092/ ./bin/ubuntuone-syncdaemon --debug
>
>The syncdaemon debug log shows that the authentication fails when using the
>new branches, and I'm able to authenticate using the system's syncdaemon, so
>there must be something that's working differently.

Yep. It turns out that I was confused by the confusing differences in
nomenclature. python-oauth uses the older terms "consumer" and "access token"
while python-oauthlib uses the RFC 5849 terms "client" and "resource owner".
When doing the conversion between the two regimes, I got them backwards and
was passing the first four arguments to oauthlib's Client constructor in the
reverse order.

This should now be fixed. I've updated the LP: #1077092 merge proposal branch
for the correct order in client.py, and it looks to me like both the tests
pass, and the cross-branch test you describe above successfully connects.

Hopefully this resolves any outstanding issues with the two branches, but
please let me know if it's still not working!

> Let's start by disabling the dbus auto-restart of the ubuntuone-syncdaemon on
> a production system. You have to do this or the `u1sdtool -q` command is
> ineffectual. It does quit the daemon, but dbus re-activates it so the one in
> bin/ubuntuone-syncdaemon will never be able to start.
>
> The easiest way I found for doing this was to comment out the Name and Exec
> lines in /usr/share/dbus-1/services/com.ubuntuone.SyncDaemon.service, then
> kill -HUP dbus to re-read its service files. Now when you `u1sdtool -q`, it
> kills the running syncdaemon and dbus won't restart it.

Thanks a lot for researching the workaround for this!
I'm ashamed to admit the autorestarting of syncdaemon has been a long standing bug that we've never prioritized.
Sorry again about that.

> Hopefully this resolves any outstanding issues with the two branches, but
> please let me know if it's still not working!

It's connecting perfectly now, thanks again for working on all these branches.

works ok IRL for me

Am just putting a negative vote at the moment, as we can't immediately land this, as it will break ubuntuone-client trunk, and as we need to fix the issue with getting branches landed there, due to squid (ncsa_auth) crashing on raring.

NP. Will this (and the other branches) get auto-merged once trunk is fixed, or do you need me to do anything else?

I'll handle getting it merged, once the squid related kinks are worked around, as it's already been approved.

The attempt to merge lp:~barry/ubuntuone-storage-protocol/lp1077092 into lp:ubuntuone-storage-protocol failed. Below is the output from the failed tests.

running build

*** Cannot find protoc; is the protobuf-compiler package installed?