Skip to content
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

[iron] Propagate "custom_data" and "ros_distro" in to the metadata.yaml file during re-indexing (backport #1700) #1711

Merged
merged 3 commits into from
Jun 23, 2024

Conversation

mergify[bot]
Copy link

@mergify mergify bot commented Jun 12, 2024

Propagates the custom_data and ros_distro fields from the underlying storage into the metadata.yaml file during re-indexing.

  • Previously this data was not extracted from the storage and the subsequent fields in metadata.yaml were left as default.
  • The implementation in this PR uses the custom_data and ros_distro metadata from the newest storage file contained within the log as custom data can be modified in the storage API between successive split files.

This fixes #1699


This is an automatic backport of pull request #1700 done by Mergify.

… during re-indexing (#1700)

* propagate custom data and ros_distro during reindexing
Signed-off-by: Cole Tucker

Signed-off-by: coalman321 <[email protected]>

* fix ament uncrustify issue

Signed-off-by: coalman321 <[email protected]>

* only update metadata if size is nonzero

Signed-off-by: coalman321 <[email protected]>

* fix uncrustify issue and switch to empty

Signed-off-by: coalman321 <[email protected]>

* update reindexer test to verify custom data and ros_distro changes

Signed-off-by: coalman321 <[email protected]>

* Convert to use exact clock type for windows CI

Signed-off-by: coalman321 <[email protected]>

---------

Signed-off-by: coalman321 <[email protected]>
(cherry picked from commit 804432c)
@mergify mergify bot requested a review from a team as a code owner June 12, 2024 21:39
@mergify mergify bot requested review from emersonknapp and jhdcs and removed request for a team June 12, 2024 21:39
@MichaelOrlov MichaelOrlov changed the title Propagate "custom_data" and "ros_distro" in to the metadata.yaml file during re-indexing (backport #1700) [iron] Propagate "custom_data" and "ros_distro" in to the metadata.yaml file during re-indexing (backport #1700) Jun 12, 2024
@MichaelOrlov
Copy link
Contributor

The original PR could be only partially backported since we don't have ros_distro field in the struct BagMetadata on Iron.

@MichaelOrlov
Copy link
Contributor

MichaelOrlov commented Jun 13, 2024

Update:
Have to skip check for custom_metadata in test for mcap storage plugin.

- Rationale:
Mcap storage doesn't store serialized metadata in bag directly on Iron
and Humble. Backporting of the relevant #1423 is not possible without
updating mcap vendor package to the v1.1.0 because we have dependencies
from the mcap_reader_->metadataIndexes(); API which became available
only in the foxglove/mcap#902

Signed-off-by: Michael Orlov <[email protected]>
@MichaelOrlov
Copy link
Contributor

Pulls: #1711
Gist: https://gist.githubusercontent.com/MichaelOrlov/ade449de1c1a6750fe6c31e59ad3ff7e/raw/9c93f13c59de398cc0c356a3bb8efe7897a87b1c/ros2.repos
BUILD args: --packages-above-and-dependencies rosbag2_cpp rosbag2_tests
TEST args: --packages-above rosbag2_cpp rosbag2_tests
ROS Distro: iron
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/14075

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Linux-rhel Build Status
  • Windows Build Status

@MichaelOrlov
Copy link
Contributor

The previous CI failure on Windows was due to the infrastracture issue. Wasn't able to download QT dependencies for build.
Re-run Windows CI

  • Windows Build Status

@MichaelOrlov MichaelOrlov merged commit 9e4b115 into iron Jun 23, 2024
14 checks passed
@mergify mergify bot deleted the mergify/bp/iron/pr-1700 branch June 23, 2024 03:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants