Add configuration paramater for wait-for-secondary #1562
Conversation
lbonn
requested review from
pattivacek,
eu-siemann,
mike-sul,
xcheng-here and
kbushgit
|
@patrickvacek yes I think we can write a test with a short timeout and unavailable ip secondary and check that it returns the error we expect. |
lbonn
force-pushed
the
feat/OTA-4422/secondary-wait-timeout
branch
from
d310e11
to
03e2a4f
Compare
Codecov Report
@@ Coverage Diff @@
## master #1562 +/- ##
==========================================
+ Coverage 82.05% 82.18% +0.13%
==========================================
Files 189 189
Lines 11840 11857 +17
==========================================
+ Hits 9715 9745 +30
+ Misses 2125 2112 -13
Continue to review full report at Codecov.
|
|
@patrickvacek yes, I think this time we should wait for the test to merge actually. Sorry, should have marked it as not ready. |
It still contained `secondary_configs_dir` which has been removed. Signed-off-by: Laurent Bonnans <laurent.bonnans@here.com>
As we want to track this value on the backend, it needs to show up in our main configuration object which gets sent when aktualizr starts. Signed-off-by: Laurent Bonnans <laurent.bonnans@here.com>
Signed-off-by: Laurent Bonnans <laurent.bonnans@here.com>
lbonn
force-pushed
the
feat/OTA-4422/secondary-wait-timeout
branch
3 times, most recently
from
245f67e
to
8cea082
Compare
lbonn
force-pushed
the
feat/OTA-4422/secondary-wait-timeout
branch
from
8cea082
to
5187769
Compare
A series of operations are needed after all the install steps are attempted and it's easy to miss one. The common post-processing is now placed at the end of a lambda call, so that nothing can be omitted. Signed-off-by: Laurent Bonnans <laurent.bonnans@here.com>
Signed-off-by: Laurent Bonnans <laurent.bonnans@here.com>
| bool drop_targets = false; | ||
|
|
||
| std::tie(r, raw_report, drop_targets) = [this, &updates, | ||
| &correlation_id]() -> std::tuple<result::Install, std::string, bool> { |
There was a problem hiding this comment.
This is a great idea and should really make things easier to deal with. Took me a minute to follow it, though. I think part of it is that the lambda is rather large. Would it perhaps make it easier to just make it a proper function? Or would that be unnecessary? I realize most of this function is the lambda now, so maybe that wouldn't really help things...
There was a problem hiding this comment.
I tend to agree actually but my bad excuse is that I don't really know how to name this function, it's really doing the bulk of the work...
The do { ... break; } while(0); idiom was considered but a return is in a nested loop, so it would not work.
Another note is that return std::make_tuple(...) can be changed to return {...} from c++14 http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2015/n4387.html. The first version was written in this style but it tripped on xenial.
As we want to track this value on the backend, it needs to show up in
our main configuration object which gets sent when aktualizr starts.