dlb: optimize auto detect logic#23465
Conversation
daixiang0
force-pushed
the
detect
branch
7 times, most recently
from
41d6894 to
277d5c8
Compare
|
I want to create a fake file under Try the below ways:
Both |
Signed-off-by: Loong <loong.dai@intel.com>
|
@adisuissa @wrowe could you help review? |
adisuissa
left a comment
adisuissa
left a comment
There was a problem hiding this comment.
LGTM, just left a question.
RE how to test this, I don't think the test should create "/dev/..." files.
Can you refactor the code so it reads it from a different location? Then the test will use the functions from https://github.com/envoyproxy/envoy/blob/main/test/test_common/environment.h to create the temp files.
|
@adisuissa I have refactored it and added new tests, please review and merge if possible. |
|
@adisuissa please review again. |
adisuissa
left a comment
adisuissa
left a comment
There was a problem hiding this comment.
LGTM, left a comment for future reference.
| // The dir should always be "/dev" in production. | ||
| // For test it is a temporary directory. | ||
| // Return Dlb device id, absl::nullopt means error. | ||
| static absl::optional<uint> detectDlbDevice(const uint config_id, const std::string& dir) { |
There was a problem hiding this comment.
nit: you can use absl::StatusOr as it allows you to specify the error.
There was a problem hiding this comment.
Sorry for slow response due to sickness, I will optimize it in the next PR.
2 participants
Signed-off-by: Loong loong.dai@intel.com
The default of type uint32 in protobuf is 0, so the
elsefield can not run into.Fix the above issue and add tests to cover some cases.
DLB mock functions are in development, so only test the config logic for now.
Commit Message:
Additional Description:
Risk Level:
Testing:
Docs Changes:
Release Notes:
Platform Specific Features:
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Fixes commit #PR or SHA]
[Optional Deprecated:]
[Optional API Considerations:]