Merge lp://qastaging/~cmiller/desktopcouch/service-must-not-call-self-over-dbus into lp://qastaging/desktopcouch

Proposed by Chad Miller
Status: Merged
Approved by: Natalia Bidart
Approved revision: 268
Merged at revision: 264
Proposed branch: lp://qastaging/~cmiller/desktopcouch/service-must-not-call-self-over-dbus
Merge into: lp://qastaging/desktopcouch
Diff against target: 336 lines (+82/-34)
6 files modified
desktopcouch/application/plugins/__init__.py (+2/-2)
desktopcouch/application/plugins/tests/test_plugins.py (+6/-1)
desktopcouch/application/plugins/tests/test_ubuntuone_pairing.py (+22/-15)
desktopcouch/application/plugins/ubuntuone_pairing.py (+17/-11)
desktopcouch/application/service.py (+4/-4)
desktopcouch/application/tests/test_service.py (+31/-1)
To merge this branch: bzr merge lp://qastaging/~cmiller/desktopcouch/service-must-not-call-self-over-dbus
Reviewer Review Type Date Requested Status
Natalia Bidart (community) Approve
Eric Casteleijn (community) Approve
Review via email: mp+47262@code.qastaging.launchpad.net

Commit message

When running plugin code in initializing the desktopcouch service, do not call any functions that require that the service be running and answering DBus method calls. In particular, we already know the port couchdb is listening on, so do not ask for that via DBus client call, but explicity pass it into the plugin function calls. (LP: #706939)

To post a comment you must log in.
Revision history for this message
Natalia Bidart (nataliabidart) wrote :

Tests are not passing in this branch (and they are in trunk, just confirmed). Errors are:

http://pastebin.ubuntu.com/557691/

Revision history for this message
Natalia Bidart (nataliabidart) :
review: Needs Fixing
Revision history for this message
Roberto Alsina (ralsina) wrote :

I get lots of test errors too, but not the same :-(

https://pastebin.canonical.com/42264/

261. By Chad Miller

Use port also in tests, silly.

262. By Chad Miller

Pass the port to be used in tests.

Migration and test-pairing still fail.

263. By Chad Miller

Give up on mocker.replace, and instead pass optional test arguments.

264. By Chad Miller

PEP8

265. By Chad Miller

Fix tests.

Revision history for this message
Eric Casteleijn (thisfred) wrote :
Download full text (3.3 KiB)

I like the solution of explicitly passing the port where necessary.

Woohoo, no more erroring/failing tests!

I still see this in the test feedback, which looks less than ideal, but probably wholly unrelated to this branch:

desktopcouch.application.pair.tests.test_network_io
  TestNetworkIO
    test_successful_lifespan ... INFO:ListenForInvitations:local port to receive invitations is 33214
DEBUG:SendInvitationFactory:initialized
Traceback (most recent call last):
  File "/home/eric/canonical/desktopcouch/service-must-not-call-self-over-dbus/desktopcouch/application/plugins/ubuntuone_pairing.py", line 90, in listen_to_dbus
    dbus_interface=iface)
  File "/usr/lib/pymodules/python2.6/dbus/bus.py", line 151, in add_signal_receiver
    path, **keywords)
  File "/usr/lib/pymodules/python2.6/dbus/connection.py", line 382, in add_signal_receiver
    self._require_main_loop()
RuntimeError: To make asynchronous calls, receive signals or export objects, D-Bus connections must be attached to a main loop by passing mainloop=... to the constructor or calling dbus.set_default_main_loop(...)
Traceback (most recent call last):
  File "/home/eric/canonical/desktopcouch/service-must-not-call-self-over-dbus/desktopcouch/application/plugins/ubuntuone_pairing.py", line 90, in listen_to_dbus
    dbus_interface=iface)
  File "/usr/lib/pymodules/python2.6/dbus/bus.py", line 151, in add_signal_receiver
    path, **keywords)
  File "/usr/lib/pymodules/python2.6/dbus/connection.py", line 382, in add_signal_receiver
    self._require_main_loop()
RuntimeError: To make asynchronous calls, receive signals or export objects, D-Bus connections must be attached to a main loop by passing mainloop=... to the constructor or calling dbus.set_default_main_loop(...)
DEBUG:SendInvitationProtocol:initialized

I see these pep8 issues also, but I believe most if not all have been fixed on trunk, so remerging should take care of that:

./desktopcouch/application/service.py:160:26: E261 at least two spaces before inline comment
./desktopcouch/application/start_local_couchdb.py:117:1: E302 expected 2 blank lines, found 1
./desktopcouch/application/platform/windows/base_dirs.py:38:74: W291 trailing whitespace
./desktopcouch/application/platform/windows/base_dirs.py:48:18: W291 trailing whitespace
./desktopcouch/application/platform/windows/base_dirs.py:62:18: W291 trailing whitespace
./desktopcouch/application/platform/windows/base_dirs.py:76:24: E211 whitespace before '['
./desktopcouch/application/platform/windows/base_dirs.py:78:37: E211 whitespace before '['
./desktopcouch/application/platform/windows/base_dirs.py:80:24: E211 whitespace before '['
./desktopcouch/application/platform/windows/keyring.py:160:48: W291 trailing whitespace
./desktopcouch/application/platform/windows/keyring.py:168:68: W292 no newline at end of file
./desktopcouch/application/platform/windows/tests/test_base_dirs.py:90:1: W391 blank line at end of file
./desktopcouch/application/plugins/ubuntuone_pairing.py:88:59: W291 trailing whitespace
./desktopcouch/application/replication_services/ubuntuone.py:66:51: W291 trailing whitespace
./desktopcouch/application/replication_services/ubuntuone.py:67:31: W29...

Read more...

review: Approve
266. By Chad Miller

Merge with trunk to fix conflicts.

267. By Chad Miller

pep8, trailing whitespace.

268. By Chad Miller

Er, I can't find a way this could swallow exceptions now.

Revision history for this message
Natalia Bidart (nataliabidart) wrote :

I filled bug #711208 since I can't have the service running withing this branch.

Can you please advice if the error is related to the branch or not? Thanks!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches