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

Send config at boot #1474

Merged
merged 7 commits into from
Dec 10, 2019
Merged

Send config at boot #1474

merged 7 commits into from
Dec 10, 2019

Conversation

lbonn
Copy link
Contributor

@lbonn lbonn commented Dec 5, 2019

No description provided.

@lbonn
Remove a duplicate include
6bc70c8 
Signed-off-by: Laurent Bonnans <laurent.bonnans@here.com>
@lbonn lbonn force-pushed the feat/OTA-4166/aktualizr-send-config branch 2 times, most recently from bbac44a to dd7c9e0 Compare December 6, 2019 13:22
@codecov-io
Copy link

codecov-io commented Dec 6, 2019 •
edited

Codecov Report

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

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #1474   +/-   ##
=========================================
  Coverage          ?   80.53%           
=========================================
  Files             ?      184           
  Lines             ?    11103           
  Branches          ?        0           
=========================================
  Hits              ?     8942           
  Misses            ?     2161           
  Partials          ?        0
Impacted Files Coverage Δ
src/libaktualizr/telemetry/telemetryconfig.h 100% <ø> (ø)
src/libaktualizr/primary/reportqueue.cc 100% <ø> (ø)
src/libaktualizr/http/httpclient.h 100% <ø> (ø)
src/libaktualizr/http/httpinterface.h 100% <ø> (ø)
src/libaktualizr/primary/sotauptaneclient.h 100% <ø> (ø)
src/libaktualizr/telemetry/telemetryconfig.cc 100% <100%> (ø)
src/libaktualizr/primary/sotauptaneclient.cc 88.71% <77.77%> (ø)
src/libaktualizr/http/httpclient.cc 95.02% <97.67%> (ø)

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 da7d9bb...bc7e9e1. Read the comment docs.

zabbal
zabbal reviewed Dec 9, 2019
View reviewed changes
Copy link
Contributor

@zabbal zabbal left a comment •
edited

Choose a reason for hiding this comment

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

Seem like this can be merge independently from the rest of PR?
Doh - I mean the first commits dealing with curl/http workarounds.

zabbal
zabbal reviewed Dec 9, 2019
View reviewed changes
src/libaktualizr/telemetry/telemetryconfig.h
#endif // TELEMETRY_TELEMETRY_CONFIG_H_
Copy link
Contributor

Choose a reason for hiding this comment

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

Not specific to this particular PR, just reminded about thing I wanted to ask: is there reason why we don't use "#pragma once"?

Copy link
Contributor

Choose a reason for hiding this comment

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

It's not in the standard, so why should we use it?

Copy link
Contributor

Choose a reason for hiding this comment

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

It's so widely supported I though it's part of the standard already. Good to know.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, according to wikipedia it's basically supported everywhere.

But I suppose the main reason we don't use it, is just plain inertia: the benefits might or might not be offseted by the amount of work it takes to convert existing files and check that it works well for all our use cases.

@lbonn
Remove a foolproof trick in httpclient.h
002b643 
It does not work

Signed-off-by: Laurent Bonnans <laurent.bonnans@here.com>
@lbonn
Remove a duplicate curl workaround
8a77fc0 
Already done in curlDupHandleWrapper()

Signed-off-by: Laurent Bonnans <laurent.bonnans@here.com>
@lbonn
Allow HttpClient to send something else than json
fa9371e 
Signed-off-by: Laurent Bonnans <laurent.bonnans@here.com>
@lbonn
Send aktualizr configuration to backend at startup
beb740c 
Signed-off-by: Laurent Bonnans <laurent.bonnans@here.com>
@lbonn
Add a switch for configuration reporting
0145196 
Signed-off-by: Laurent Bonnans <laurent.bonnans@here.com>
@lbonn lbonn force-pushed the feat/OTA-4166/aktualizr-send-config branch from dd7c9e0 to 0145196 Compare December 9, 2019 10:32
kbushgit
kbushgit previously approved these changes Dec 9, 2019
View reviewed changes
@eu-siemann eu-siemann force-pushed the feat/OTA-4166/aktualizr-send-config branch from 657dfd1 to c2ea343 Compare December 9, 2019 13:44
@lbonn lbonn force-pushed the feat/OTA-4166/aktualizr-send-config branch from c2ea343 to 0145196 Compare December 9, 2019 15:00
@lbonn
Copy link
Contributor Author

lbonn commented Dec 9, 2019

@eu-smirnov I've remove the default implementation of the json api in HttpInterface. Overloading in HttpClient and HttpFake is now more natural.

@lbonn lbonn force-pushed the feat/OTA-4166/aktualizr-send-config branch from c99351b to c781679 Compare December 9, 2019 16:29
@lbonn
Remove default implementations in HttpInterface
bc7e9e1 
Keeping a pure interface simplifies things.

Signed-off-by: Laurent Bonnans <laurent.bonnans@here.com>
@lbonn lbonn force-pushed the feat/OTA-4166/aktualizr-send-config branch from c781679 to bc7e9e1 Compare December 10, 2019 09:37
@lbonn
Copy link
Contributor Author

lbonn commented Dec 10, 2019

A very stupid copy-paste typo was failing the tests... It should be good by now.

@lbonn lbonn merged commit 0d138a9 into master Dec 10, 2019
@lbonn lbonn deleted the feat/OTA-4166/aktualizr-send-config branch December 10, 2019 10:59
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@zabbal zabbal zabbal left review comments

@pattivacek pattivacek pattivacek left review comments

@eu-siemann eu-siemann eu-siemann approved these changes

@kbushgit kbushgit kbushgit left review comments

@mike-sul mike-sul Awaiting requested review from mike-sul

@Zee314159 Zee314159 Awaiting requested review from Zee314159

@xcheng-here xcheng-here Awaiting requested review from xcheng-here

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@lbonn @codecov-io @zabbal @pattivacek @eu-siemann @kbushgit