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

Better "could not put manifest" error #1137

Merged
merged 1 commit into from
Mar 14, 2019
Merged

Conversation

lbonn
Copy link
Contributor

@lbonn lbonn commented Mar 14, 2019

  • stop logging it when the manifest put is inhibited by a pending install
  • more detailed output when it couldn't be sent to the server

- stop logging it when the manifest put is inhibited by a pending install
- more detailed output when it couldn't be sent to the server

Signed-off-by: Laurent Bonnans <laurent.bonnans@here.com>
@codecov-io
Copy link

Codecov Report

Merging #1137 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1137   +/-   ##
=======================================
  Coverage   76.17%   76.17%           
=======================================
  Files         164      164           
  Lines        9869     9869           
=======================================
  Hits         7518     7518           
  Misses       2351     2351
Impacted Files Coverage Δ
src/libaktualizr/primary/sotauptaneclient.cc 87% <100%> (ø) ⬆️

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 ffbed75...fe724cb. Read the comment docs.

Copy link
Collaborator

@pattivacek pattivacek left a comment

Choose a reason for hiding this comment

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

+1.

@pattivacek pattivacek merged commit 10b39ee into master Mar 14, 2019
@pattivacek pattivacek deleted the fix/manifest-error-logging branch March 14, 2019 12:46
@@ -1227,6 +1226,8 @@ bool SotaUptaneClient::putManifestSimple() {
storage->clearInstallationResults();
return true;
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
At first glance it looks like the manifest assembling is redundant if there are pending updates, so I'd move line #1217 just after `if (hasPendingUpdates())` statement.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point, but I'd need to double check. I have vague memories about this function having or used to having side-effects.

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