Merge lp://staging/~charlesk/keeper/connection-helper into lp://staging/keeper/devel

Proposed by Charles Kerr
Status: Merged
Merged at revision: 102
Proposed branch: lp://staging/~charlesk/keeper/connection-helper
Merge into: lp://staging/keeper/devel
Diff against target: 2093 lines (+676/-498)
48 files modified
include/client/client.h (+1/-1)
include/helper/backup-helper.h (+5/-8)
include/helper/helper.h (+1/-2)
include/helper/metadata.h (+1/-2)
src/cli/CMakeLists.txt (+3/-0)
src/helper/CMakeLists.txt (+2/-0)
src/helper/backup-helper.cpp (+78/-66)
src/helper/helper.cpp (+5/-7)
src/helper/metadata.cpp (+1/-5)
src/service/CMakeLists.txt (+2/-2)
src/service/app-const.h (+0/-3)
src/service/backup-choices.cpp (+1/-1)
src/service/backup-choices.h (+1/-1)
src/service/keeper-task-backup.cpp (+30/-104)
src/service/keeper-task-backup.h (+3/-3)
src/service/keeper-task.cpp (+18/-14)
src/service/keeper-task.h (+3/-5)
src/service/keeper-user.h (+1/-1)
src/service/keeper.cpp (+16/-47)
src/service/metadata-provider.h (+1/-1)
src/service/private/keeper-task_p.h (+2/-2)
src/service/restore-choices.cpp (+1/-1)
src/service/restore-choices.h (+1/-1)
src/service/task-manager.cpp (+27/-38)
src/service/task-manager.h (+3/-3)
src/storage-framework/CMakeLists.txt (+3/-1)
src/storage-framework/sf-uploader.cpp (+55/-0)
src/storage-framework/sf-uploader.h (+45/-0)
src/storage-framework/storage_framework_client.cpp (+107/-112)
src/storage-framework/storage_framework_client.h (+25/-28)
src/storage-framework/uploader.h (+44/-0)
src/tar/tar-creator.cpp (+1/-1)
src/tar/tar-creator.h (+1/-1)
src/util/CMakeLists.txt (+28/-9)
src/util/connection-helper.h (+120/-0)
tests/dbusmock/CMakeLists.txt (+12/-13)
tests/fakes/upstart/upstart-job-mock.cpp (+8/-0)
tests/integration/CMakeLists.txt (+3/-1)
tests/integration/helpers/CMakeLists.txt (+4/-2)
tests/integration/helpers/helpers-test-failure.cpp (+1/-1)
tests/integration/helpers/helpers-test.cc (+2/-2)
tests/integration/helpers/test-helpers-base.cpp (+1/-1)
tests/integration/helpers/test-helpers-base.h (+1/-1)
tests/unit/CMakeLists.txt (+3/-1)
tests/unit/helper/fake-helper.h (+0/-1)
tests/unit/tar/tar-creator-libarchive-failure-test.cpp (+3/-3)
tests/utils/file-utils.cpp (+1/-1)
tests/utils/file-utils.h (+1/-1)
To merge this branch: bzr merge lp://staging/~charlesk/keeper/connection-helper
Reviewer Review Type Date Requested Status
unity-api-1-bot continuous-integration Needs Fixing
Xavi Garcia (community) Approve
Review via email: mp+304966@code.staging.launchpad.net

Commit message

Add ConnectionHelper class to safely manage QFutures and signals

Description of the change

* Add ConnectionHelper, a class to manage connections to QFutures and one-shot signals. It will disconnect & release resources when the QFuture is finished or when the single-shot signal is called. Destroying the ConnectionHelper also disconnects & releases resources, so an object that needs to safely manage those kinds of connections can aggregate a ConnectionHelper so that connections are destroyed when the object is destroyed.

* Reimplement StorageFrameworkClient, Keeper, Helper, and Task connection management using ConnectionHelper

* move Uploader::commit() (previously StorageFrameworkClient::finish(bool)) logic from Task to BackupHelper

Other:

* fix our QObject shared pointers such that their deleters call deleteLater()

* fix minor TestHelpers::StartHelper mistest that looked for DATA_COMPLETE instead of COMPLETE; the only reason the former was reached before was because BackupHelperPrivate's constructor built a new (but empty) storage framework QLocalSocket

* fix broken email addresses

To post a comment you must log in.
Revision history for this message
unity-api-1-bot (unity-api-1-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
unity-api-1-bot (unity-api-1-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
unity-api-1-bot (unity-api-1-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
unity-api-1-bot (unity-api-1-bot) wrote :

FAILED: Continuous integration, rev:121
https://jenkins.canonical.com/unity-api-1/job/lp-keeper-ci/41/
Executed test runs:
    FAILURE: https://jenkins.canonical.com/unity-api-1/job/build/565/console
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-0-fetch/571
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=vivid+overlay/399
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=vivid+overlay/399/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=xenial+overlay/399
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=xenial+overlay/399/artifact/output/*zip*/output.zip
    FAILURE: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=yakkety/399/console
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=vivid+overlay/399
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=vivid+overlay/399/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=xenial+overlay/399
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=xenial+overlay/399/artifact/output/*zip*/output.zip
    FAILURE: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=yakkety/399/console
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=vivid+overlay/399
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=vivid+overlay/399/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=xenial+overlay/399
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=xenial+overlay/399/artifact/output/*zip*/output.zip
    FAILURE: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=yakkety/399/console

Click here to trigger a rebuild:
https://jenkins.canonical.com/unity-api-1/job/lp-keeper-ci/41/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
unity-api-1-bot (unity-api-1-bot) wrote :

PASSED: Continuous integration, rev:123
https://jenkins.canonical.com/unity-api-1/job/lp-keeper-ci/42/
Executed test runs:
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build/566
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-0-fetch/572
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=vivid+overlay/400
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=vivid+overlay/400/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=xenial+overlay/400
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=xenial+overlay/400/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=yakkety/400
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=yakkety/400/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=vivid+overlay/400
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=vivid+overlay/400/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=xenial+overlay/400
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=xenial+overlay/400/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=yakkety/400
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=yakkety/400/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=vivid+overlay/400
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=vivid+overlay/400/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=xenial+overlay/400
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=xenial+overlay/400/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=yakkety/400
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=yakkety/400/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://jenkins.canonical.com/unity-api-1/job/lp-keeper-ci/42/rebuild

review: Approve (continuous-integration)
Revision history for this message
unity-api-1-bot (unity-api-1-bot) wrote :

FAILED: Continuous integration, rev:125
https://jenkins.canonical.com/unity-api-1/job/lp-keeper-ci/43/
Executed test runs:
    FAILURE: https://jenkins.canonical.com/unity-api-1/job/build/567/console
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-0-fetch/573
    FAILURE: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=vivid+overlay/401/console
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=xenial+overlay/401
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=xenial+overlay/401/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=yakkety/401
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=yakkety/401/artifact/output/*zip*/output.zip
    FAILURE: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=vivid+overlay/401/console
    FAILURE: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=xenial+overlay/401/console
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=yakkety/401
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=yakkety/401/artifact/output/*zip*/output.zip
    FAILURE: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=vivid+overlay/401/console
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=xenial+overlay/401
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=xenial+overlay/401/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=yakkety/401
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=yakkety/401/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://jenkins.canonical.com/unity-api-1/job/lp-keeper-ci/43/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
unity-api-1-bot (unity-api-1-bot) wrote :

PASSED: Continuous integration, rev:128
https://jenkins.canonical.com/unity-api-1/job/lp-keeper-ci/44/
Executed test runs:
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build/569
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-0-fetch/575
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=vivid+overlay/403
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=vivid+overlay/403/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=xenial+overlay/403
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=xenial+overlay/403/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=yakkety/403
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=yakkety/403/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=vivid+overlay/403
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=vivid+overlay/403/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=xenial+overlay/403
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=xenial+overlay/403/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=yakkety/403
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=yakkety/403/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=vivid+overlay/403
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=vivid+overlay/403/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=xenial+overlay/403
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=xenial+overlay/403/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=yakkety/403
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=yakkety/403/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://jenkins.canonical.com/unity-api-1/job/lp-keeper-ci/44/rebuild

review: Approve (continuous-integration)
Revision history for this message
unity-api-1-bot (unity-api-1-bot) wrote :

PASSED: Continuous integration, rev:131
https://jenkins.canonical.com/unity-api-1/job/lp-keeper-ci/45/
Executed test runs:
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build/570
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-0-fetch/576
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=vivid+overlay/404
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=vivid+overlay/404/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=xenial+overlay/404
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=xenial+overlay/404/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=yakkety/404
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=yakkety/404/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=vivid+overlay/404
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=vivid+overlay/404/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=xenial+overlay/404
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=xenial+overlay/404/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=yakkety/404
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=yakkety/404/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=vivid+overlay/404
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=vivid+overlay/404/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=xenial+overlay/404
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=xenial+overlay/404/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=yakkety/404
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=yakkety/404/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://jenkins.canonical.com/unity-api-1/job/lp-keeper-ci/45/rebuild

review: Approve (continuous-integration)
Revision history for this message
Xavi Garcia (xavi-garcia-mena) wrote :

Very nice addition, Charles. Looks good to me :)

I only have questions for a couple of things:

- I think the ConnectionHelper could be very useful for other projects and also... taking into account that keeper runs as a service: would it make sense to change the type of the tag to something that offers a bigger range? (now it is unsinged int). Just thinking on services that run for a lot of time and will make a lot of connections. The tag always increments for new connections...
I also don't know if we could remove the connection from the map when we disconnect the connection at the remember method.

- Would it make sense to add code for the "cancelled" signal in QFutureWatcher for connect_future? I don't know if storage-framework cancels futures at all, though. In fact that was not managed by keeper at all previously.

I'm approving the branch as it looks really good, but I'm leaving here the questions. In fact they can be added in a future branch.

review: Approve
Revision history for this message
unity-api-1-bot (unity-api-1-bot) wrote :

FAILED: Autolanding.
More details in the following jenkins job:
https://jenkins.canonical.com/unity-api-1/job/lp-keeper-autoland/6/
Executed test runs:
    FAILURE: https://jenkins.canonical.com/unity-api-1/job/build/579/console
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-0-fetch/585
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=vivid+overlay/411
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=vivid+overlay/411/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=xenial+overlay/411
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=xenial+overlay/411/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=yakkety/411
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=yakkety/411/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=vivid+overlay/411
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=vivid+overlay/411/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=xenial+overlay/411
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=xenial+overlay/411/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=yakkety/411
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=yakkety/411/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=vivid+overlay/411
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=vivid+overlay/411/artifact/output/*zip*/output.zip
    FAILURE: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=xenial+overlay/411/console
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=yakkety/411
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=yakkety/411/artifact/output/*zip*/output.zip

review: Needs Fixing (continuous-integration)

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

to all changes: