-
Notifications
You must be signed in to change notification settings - Fork 61
Refact/ota 2487/ipsecondaries initialization #1198
Refact/ota 2487/ipsecondaries initialization #1198
Conversation
mike-sul
commented
May 1, 2019
- Add ability to specify params that are common across secondaries of the same type (connection wait timeout);
- Primary waits for a connection from Secondary if it fails to connect to it. Secondary tries to connect to Primary at a startup time.
Signed-off-by: Mike Sul <[email protected]>
e8e7bf2
to
874391a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks pretty good, I think. I don't think I have any significant comments to make.
@@ -7,6 +7,7 @@ | |||
|
|||
namespace Primary { | |||
|
|||
const char* const IPSecondariesConfig::Type = "IP"; | |||
const char* const IPSecondaryConfig::Type = "ip"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems slightly odd to have two constants that are seemingly the same except for subtle differences in count and capitalization. Are these both really necessary?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no, they aren't, I've removed IPSecondaryConfig::Type
@@ -10,20 +10,30 @@ | |||
|
|||
namespace Uptane { | |||
|
|||
std::shared_ptr<Uptane::SecondaryInterface> IpUptaneSecondary::create(const std::string& address, unsigned short port) { | |||
std::pair<bool, std::shared_ptr<Uptane::SecondaryInterface>> IpUptaneSecondary::connectAndcreate( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pretty insignificant, but we generally like camelCase, so this should be "connectAndCreate".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
agree
874391a
to
3aa0366
Compare
…ct to it Signed-off-by: Mike Sul <[email protected]>
3aa0366
to
a7b825d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks ready to go to me! Travis failure is just in the CI upload; tests passed. That's getting annoying but won't stop me merging this now.