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

Create new "aktualizr-get" command #1276

Merged
merged 2 commits into from
Aug 14, 2019

Conversation

doanac
Copy link
Collaborator

@doanac doanac commented Aug 1, 2019

I find myself carefully crafting wget commands to debug issues
between my device and device-gateway. This has gotten more complex
now that my team is using HSMs. This adds a simple tool to work
a bit like wget.

Signed-off-by: Andy Doan andy@foundries.io

I find myself carefully crafting `wget` commands to debug issues
between my device and device-gateway. This has gotten more complex
now that my team is using HSMs. This adds a simple tool to work
a bit like wget.

Signed-off-by: Andy Doan <andy@foundries.io>
@doanac
Copy link
Collaborator Author

doanac commented Aug 1, 2019

Feel free to reject this. Its easy enough for my team to manage out-of-tree. However, I thought you guys might find it useful also.

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.

Seems cool and can't hurt! CI is complaining about some static checks but it looks pretty close to mergeable otherwise.

namespace bpo = boost::program_options;

bpo::variables_map parse_options(int argc, char *argv[]) {
bpo::options_description description("aktualizr-get command line options");
Copy link
Collaborator

Choose a reason for hiding this comment

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

Might be useful to either print something here or at least have a comment to explain what the tool is for.

int r = EXIT_FAILURE;
try {
if (geteuid() != 0) {
LOG_WARNING << "\033[31mRunning as non-root and may not work as expected!\033[0m\n";
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is that true? I don't think it applies to this tool.

@codecov-io
Copy link

codecov-io commented Aug 5, 2019

Codecov Report

❗ No coverage uploaded for pull request base (master@a0e5959). Click here to learn what that means.
The diff coverage is 20%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #1276   +/-   ##
=========================================
  Coverage          ?   78.75%           
=========================================
  Files             ?      177           
  Lines             ?    10417           
  Branches          ?        0           
=========================================
  Hits              ?     8204           
  Misses            ?     2213           
  Partials          ?        0
Impacted Files Coverage Δ
src/aktualizr_get/main.cc 0% <0%> (ø)
src/aktualizr_get/get.cc 83.33% <83.33%> (ø)

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 a0e5959...0b0cff6. Read the comment docs.

@doanac
Copy link
Collaborator Author

doanac commented Aug 5, 2019

@patrickvacek - good points. fixed now.

@pattivacek
Copy link
Collaborator

Looks good, but CI is still complaining about a compilation error.

@pattivacek
Copy link
Collaborator

Looks like you fixed the compilation error, but accidentally changed the version of tuf-test-vectors, so now that is failing. If you undo that bit, it'll probably succeed.

Signed-off-by: Andy Doan <andy@foundries.io>
@doanac
Copy link
Collaborator Author

doanac commented Aug 13, 2019

git commit -a --amend can be evil. sorry about that.

@pattivacek
Copy link
Collaborator

Travis is timing out, which is annoying, but I tested it locally and it's fine.

@pattivacek pattivacek merged commit 79f4342 into advancedtelematic:master Aug 14, 2019
@doanac doanac deleted the aktualizr-get branch August 14, 2019 14:06
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

3 participants