-
Notifications
You must be signed in to change notification settings - Fork 800
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[22735] Make the machine id the physical data host info #5592
Conversation
3f34e53
to
7fba2e3
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 great! Although I believe we should check if the machine-id
is empty before using it to set the property, and fall back to the previous method in case it is empty. This might happens in certain docker environments.
Signed-off-by: Juanjo Garcia <[email protected]>
b56405e
to
7a70cd0
Compare
Signed-off-by: Juanjo Garcia <[email protected]>
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.
LGTM with green CI
* Refs #22575: Make the machine id the physical data host info Signed-off-by: Juanjo Garcia <[email protected]> * Refs #22735: Added reviwer suggestions Signed-off-by: Juanjo Garcia <[email protected]> --------- Signed-off-by: Juanjo Garcia <[email protected]> Signed-off-by: RookieCLY <[email protected]>
* Refs #22575: Make the machine id the physical data host info Signed-off-by: Juanjo Garcia <[email protected]> * Refs #22735: Added reviwer suggestions Signed-off-by: Juanjo Garcia <[email protected]> --------- Signed-off-by: Juanjo Garcia <[email protected]> Signed-off-by: RookieCLY <[email protected]>
This PR modifies the info that gets fed into the
fastdds.physical_data
host element, themachine_id
instead of thehost_id
as it was before.Contributor Checklist
versions.md
file (if applicable).Reviewer Checklist