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

Check & create database dirs before writing bags #1133

Merged
merged 4 commits into from
Oct 25, 2022

Conversation

homalozoa
Copy link

@homalozoa homalozoa commented Oct 20, 2022

Ref: #450

This feature is necessary for using writer to create bags in foxy distribution.

  1. Port db checking & creation to sequential_writer and sequential_compression_writer.
  2. Port test code for sequential_writer and sequential_compression_writer.
  3. Remove db dir creation from verb record.py in ros2bag.
  4. Add .vscode, install, log dirs in .gitignore.

Signed-off-by: homalozoa nx.tardis@gmail.com

Signed-off-by: homalozoa <nx.tardis@gmail.com>
Signed-off-by: homalozoa <nx.tardis@gmail.com>
1. Port dir creation to sequential_compression_writer
2. Remove dir before test db creation

Signed-off-by: homalozoa <nx.tardis@gmail.com>
* Database directories' creation will happend while writer open database
  file. So there is no need to create db in record.py

Signed-off-by: homalozoa <nx.tardis@gmail.com>
@homalozoa homalozoa changed the title Check & create dir before writing bag if necessary Check & create db dir before writing bag if necessary Oct 21, 2022
@homalozoa homalozoa changed the title Check & create db dir before writing bag if necessary Check & create db dir before writing bag. Oct 21, 2022
@homalozoa homalozoa changed the title Check & create db dir before writing bag. Check & create db dir before writing bags Oct 21, 2022
@homalozoa homalozoa changed the title Check & create db dir before writing bags Check & create database dirs before writing bags Oct 21, 2022
Copy link
Contributor

@MichaelOrlov MichaelOrlov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@homalozoa LGTM with green CI.
Thanks for the backport.

@MichaelOrlov
Copy link
Contributor

Gist: https://gist.githubusercontent.com/MichaelOrlov/c54ef5b9ccbff8d711733b4a03ed5f8f/raw/2fb56f6b8be756c6de4c1b6b5cf632f5defeb1c5/ros2.repos
BUILD args: --packages-above-and-dependencies ros2bag rosbag2_compression rosbag2_cpp
TEST args: --packages-above ros2bag rosbag2_compression rosbag2_cpp
ROS Distro: foxy
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/11012

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

@homalozoa
Copy link
Author

Gist: https://gist.githubusercontent.com/MichaelOrlov/c54ef5b9ccbff8d711733b4a03ed5f8f/raw/2fb56f6b8be756c6de4c1b6b5cf632f5defeb1c5/ros2.repos BUILD args: --packages-above-and-dependencies ros2bag rosbag2_compression rosbag2_cpp TEST args: --packages-above ros2bag rosbag2_compression rosbag2_cpp ROS Distro: foxy Job: ci_launcher ci_launcher ran: https://ci.ros2.org/job/ci_launcher/11012

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

The CI check broke during setting up the virtual environment. Could you trigger it again? @MichaelOrlov Thank you!

@MichaelOrlov
Copy link
Contributor

Trying to re-run Linux CI build with focal distro

  • Linux Build Status

@MichaelOrlov
Copy link
Contributor

re-run Linux-aarch64 CI build with focal distro

  • Linux-aarch64 Build Status

@MichaelOrlov
Copy link
Contributor

Re-run Windows CI job

  • Windows Build Status

@MichaelOrlov
Copy link
Contributor

Re-run Windows CI job one more time

  • Windows Build Status

@MichaelOrlov
Copy link
Contributor

CI Windows job failure is unrelated for the changes in this PR and is a known issue which was attempted to solve in #466 and #525

@MichaelOrlov MichaelOrlov merged commit 197b094 into ros2:foxy Oct 25, 2022
@homalozoa homalozoa deleted the fix__write_bag_miss_dir branch October 25, 2022 07:19
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