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

Fix/misc #1180

Merged
merged 2 commits into from
Apr 16, 2019
Merged

Fix/misc #1180

merged 2 commits into from
Apr 16, 2019

Conversation

pattivacek
Copy link
Collaborator

Things found while doing other things.

It did not match the format of empty targets that the server actually
sends us.

Signed-off-by: Patrick Vacek <patrickvacek@gmail.com>
@@ -628,7 +628,7 @@ std::pair<bool, Uptane::Target> SotaUptaneClient::downloadImage(Uptane::Target t
}
}
if (!success) {
LOG_ERROR << "Download unsuccessful after " << tries << " attempts.";
LOG_ERROR << "Download unsuccessful after " << tries - 1 << " attempts.";
Copy link
Contributor

Choose a reason for hiding this comment

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

How about for (; tries < max_tries; tries++) ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done.

@codecov-io
Copy link

codecov-io commented Apr 15, 2019

Codecov Report

Merging #1180 into master will decrease coverage by 0.03%.
The diff coverage is 92.85%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1180      +/-   ##
==========================================
- Coverage   78.01%   77.97%   -0.04%     
==========================================
  Files         170      170              
  Lines       10006    10013       +7     
==========================================
+ Hits         7806     7808       +2     
- Misses       2200     2205       +5
Impacted Files Coverage Δ
src/libaktualizr/primary/sotauptaneclient.cc 91.19% <100%> (ø) ⬆️
src/aktualizr_repo/director_repo.cc 91.04% <87.5%> (-0.63%) ⬇️
src/libaktualizr/package_manager/ostreemanager.cc 68.22% <0%> (-1.7%) ⬇️

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 9c5ef10...a0f1c5e. Read the comment docs.

targets_unsigned["expires"] = expiration_time_;
targets_unsigned["version"] = (targets_current["signed"]["version"].asUInt()) + 1;
targets_unsigned["targets"] = Json::objectValue;
if (repo_type_ == Uptane::RepositoryType::Director() && correlation_id_ != "") {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this a fix for the super bug ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The super bug? The fix here is just that the generated metadata was incorrect. It had the wrong meaning of "empty".

Turns out math is hard.

Signed-off-by: Patrick Vacek <patrickvacek@gmail.com>
success = package_manager_->fetchTarget(target, *uptane_fetcher, keys, prog_cb, token);
if (success) {
break;
} else if (tries < max_tries) {
} else if (tries < max_tries - 1) {
Copy link
Contributor

Choose a reason for hiding this comment

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

So we won't sleep in the last iteration (i = 2)? My guess would be that we don't need an else if here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Right. Why bother sleeping if we've tried three times and failed? No need to sleep before moving on to the next thing.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sure. Somehow I always get confused by this simple loop.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Me too. This is at least the third time we've tried to get it right despite it being so deceptively simple.

@pattivacek pattivacek merged commit 9edf089 into master Apr 16, 2019
@pattivacek pattivacek deleted the fix/misc branch April 16, 2019 09:09
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.

5 participants