Skip to content
This repository has been archived by the owner on May 21, 2024. It is now read-only.

Increase scope of pause api #1075

Merged
merged 4 commits into from
Feb 6, 2019
Merged

Increase scope of pause api #1075

merged 4 commits into from
Feb 6, 2019

Conversation

lbonn
Copy link
Contributor

@lbonn lbonn commented Jan 29, 2019

Now succeeds at all time: pauses current download as well as the api queue.

I still need to write some tests

Copy link
Contributor

@OYTIS OYTIS left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great!

I feel like it might be useful if Aktualizr::Pause() didn't return until the download (if there is one in progress) is "really" paused, i.e. the image or the metadata about the image is committed to the storage. But that might be another user story.

@eu-siemann
Copy link
Contributor

At first glance looks great to me as well!
As we just have discussed with @OYTIS it should be fine for Pause to return immediately. In another PR we should remove the Shutdown call and introduce Abort instead. So the user who wants to terminate the application while being sure that all data is safe must call Abort. Abort will block until the Download operation saves the state and exits and all other commands are removed from the API queue.

@codecov-io
Copy link

codecov-io commented Jan 29, 2019

Codecov Report

Merging #1075 into master will increase coverage by 0.09%.
The diff coverage is 77.27%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1075      +/-   ##
==========================================
+ Coverage   75.37%   75.47%   +0.09%     
==========================================
  Files         158      158              
  Lines        9262     9263       +1     
==========================================
+ Hits         6981     6991      +10     
+ Misses       2281     2272       -9
Impacted Files Coverage Δ
src/libaktualizr/primary/events.h 100% <ø> (ø) ⬆️
src/libaktualizr/primary/sotauptaneclient.h 100% <ø> (ø) ⬆️
src/libaktualizr/primary/results.h 100% <ø> (ø) ⬆️
src/hmi_stub/main.cc 0% <0%> (ø) ⬆️
src/libaktualizr/primary/reportqueue.h 27.27% <0%> (-6.07%) ⬇️
src/libaktualizr/primary/sotauptaneclient.cc 86.43% <100%> (+0.03%) ⬆️
src/libaktualizr/primary/aktualizr.h 100% <100%> (ø) ⬆️
src/libaktualizr/primary/reportqueue.cc 100% <100%> (ø) ⬆️
src/libaktualizr/primary/aktualizr.cc 84.34% <100%> (+1.17%) ⬆️
src/libaktualizr/uptane/fetcher.cc 96.87% <100%> (ø) ⬆️
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 023c5fd...766dfed. Read the comment docs.

@lbonn lbonn force-pushed the feat/full-pause branch 2 times, most recently from 1b7410e to a58ca56 Compare February 1, 2019 11:28
@lbonn lbonn removed the not-ready label Feb 1, 2019
@lbonn
Copy link
Contributor Author

lbonn commented Feb 1, 2019

Significant changes since last review.

I also removed DownloadPaused and DownloadResumed events.

Now succeeds at all time: pauses current download as well as the api
queue.

Signed-off-by: Laurent Bonnans <laurent.bonnans@here.com>
Signed-off-by: Laurent Bonnans <laurent.bonnans@here.com>
Success or already paused or already running.

Also stop sending API events as this call is synchronous.

Signed-off-by: Laurent Bonnans <laurent.bonnans@here.com>
Signed-off-by: Laurent Bonnans <laurent.bonnans@here.com>
@lbonn lbonn merged commit 1dcb25e into master Feb 6, 2019
@lbonn lbonn deleted the feat/full-pause branch February 6, 2019 14:25
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants