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.

Sign up for GitHub

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

Jump to bottom

Store db schema version and ROS_DISTRO name in db3 files #1156

Merged
merged 4 commits into from
Nov 22, 2022
Merged

Store db schema version and ROS_DISTRO name in db3 files #1156

merged 4 commits into from
Nov 22, 2022

Conversation

MichaelOrlov
Copy link
Contributor

@MichaelOrlov
Add new tables for schema and metadata
4a3c823 
Signed-off-by: Michael Orlov <michael.orlov@apex.ai>
@MichaelOrlov
Add SqliteWrapper::table_exists(table_name)
cfeeb19 
Signed-off-by: Michael Orlov <michael.orlov@apex.ai>
@MichaelOrlov
Add get_db_schema_versio() and get_recorded_ros_distro()
274fa44 
Signed-off-by: Michael Orlov <michael.orlov@apex.ai>
@MichaelOrlov
Add unit tests for checking new and old db schema versions
5e71460 
Added following unit tests:
- `get_db_schema_version_returns_correct_value`
- `get_recorded_ros_distro_returns_correct_value`
- `check_backward_compatibility_with_schema_version_2`

Signed-off-by: Michael Orlov <michael.orlov@apex.ai>
@MichaelOrlov MichaelOrlov force-pushed the morlov/store-db-schema-in-db3-files branch from 81f77af to 5e71460 Compare November 10, 2022 06:49
@MichaelOrlov MichaelOrlov marked this pull request as ready for review November 10, 2022 06:51
@MichaelOrlov MichaelOrlov requested a review from a team as a code owner November 10, 2022 06:51
@MichaelOrlov MichaelOrlov requested review from emersonknapp, hidmic and clalancette and removed request for a team November 10, 2022 06:51
@MichaelOrlov
Copy link
Contributor Author

MichaelOrlov commented Nov 10, 2022 •
edited
Loading

@clalancette Could you please review this PR for ABI and API stability to be able to backport this PR to Humble?

@MichaelOrlov
Copy link
Contributor Author

cc: @amacneil @defunctzombie

emersonknapp
emersonknapp approved these changes Nov 22, 2022
View reviewed changes
Copy link
Collaborator

@emersonknapp emersonknapp left a comment

Choose a reason for hiding this comment

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

I'm going to approve this as fine, I don't think we need to go back and forth too much on this part of it since it doesn't affect how other storage implementations have to do anything, there are no core APIs.

I have noted in the comment stream that I agree with Adrian that I might approach it a little differently, but those opinions are non-blocking for me.

@MichaelOrlov
Copy link
Contributor Author

Gist: https://gist.githubusercontent.com/MichaelOrlov/43376b9180367dfdbc6f83ac9e0b571f/raw/9de6d51acca2cd167442395b576ed59daa25d077/ros2.repos
BUILD args: --packages-above-and-dependencies rosbag2_storage rosbag2_storage_sqlite3 rosbag2_tests
TEST args: --packages-above rosbag2_storage rosbag2_storage_sqlite3 rosbag2_tests
ROS Distro: rolling
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/11164

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

@MichaelOrlov MichaelOrlov merged commit 9498426 into rolling Nov 22, 2022
@delete-merged-branch delete-merged-branch bot deleted the morlov/store-db-schema-in-db3-files branch November 22, 2022 22:36
@MichaelOrlov
Copy link
Contributor Author

https://github.com/Mergifyio backport humble

mergify bot pushed a commit that referenced this pull request Nov 22, 2022
@MichaelOrlov @mergify
Store db schema version and ROS_DISTRO name in db3 files (#1156)
a272433 
* Add new tables for `schema` and `metadata`

Signed-off-by: Michael Orlov <michael.orlov@apex.ai>

* Add `SqliteWrapper::table_exists(table_name)`

Signed-off-by: Michael Orlov <michael.orlov@apex.ai>

* Add `get_db_schema_versio()` and `get_recorded_ros_distro()`

Signed-off-by: Michael Orlov <michael.orlov@apex.ai>

* Add unit tests for checking new and old db schema versions

Added following unit tests:
- `get_db_schema_version_returns_correct_value`
- `get_recorded_ros_distro_returns_correct_value`
- `check_backward_compatibility_with_schema_version_2`

Signed-off-by: Michael Orlov <michael.orlov@apex.ai>

Signed-off-by: Michael Orlov <michael.orlov@apex.ai>
(cherry picked from commit 9498426)

# Conflicts:
#	rosbag2_storage_default_plugins/include/rosbag2_storage_default_plugins/sqlite/sqlite_storage.hpp
#	rosbag2_storage_default_plugins/src/rosbag2_storage_default_plugins/sqlite/sqlite_storage.cpp
@mergify mergify bot mentioned this pull request Nov 22, 2022
Merged
@mergify
Copy link

mergify bot commented Nov 22, 2022

backport humble

✅ Backports have been created

MichaelOrlov added a commit that referenced this pull request Nov 22, 2022
@MichaelOrlov
Fix merge conflicts after backporting pr #1156 from rolling
8b856f5 
Signed-off-by: Michael Orlov <michael.orlov@apex.ai>
MichaelOrlov added a commit that referenced this pull request Dec 3, 2022
@mergify @MichaelOrlov
[humble] Store db schema version and ROS_DISTRO name in db3 files (ba…
b64b7b0 
…ckport #1156) (#1175)

* Store db schema version and ROS_DISTRO name in db3 files (#1156)

* Add new tables for `schema` and `metadata`

Signed-off-by: Michael Orlov <michael.orlov@apex.ai>

* Add `SqliteWrapper::table_exists(table_name)`

Signed-off-by: Michael Orlov <michael.orlov@apex.ai>

* Add `get_db_schema_versio()` and `get_recorded_ros_distro()`

Signed-off-by: Michael Orlov <michael.orlov@apex.ai>

* Add unit tests for checking new and old db schema versions

Added following unit tests:
- `get_db_schema_version_returns_correct_value`
- `get_recorded_ros_distro_returns_correct_value`
- `check_backward_compatibility_with_schema_version_2`

Signed-off-by: Michael Orlov <michael.orlov@apex.ai>

Signed-off-by: Michael Orlov <michael.orlov@apex.ai>
(cherry picked from commit 9498426)

# Conflicts:
#	rosbag2_storage_default_plugins/include/rosbag2_storage_default_plugins/sqlite/sqlite_storage.hpp
#	rosbag2_storage_default_plugins/src/rosbag2_storage_default_plugins/sqlite/sqlite_storage.cpp

* Fix merge conflicts after backporting pr #1156 from rolling

Signed-off-by: Michael Orlov <michael.orlov@apex.ai>

Signed-off-by: Michael Orlov <michael.orlov@apex.ai>
Co-authored-by: Michael Orlov <michael.orlov@apex.ai>
@ros-discourse
Copy link

This pull request has been mentioned on ROS Discourse. There might be relevant details there:

https://discourse.ros.org/t/ros-2-tsc-meeting-minutes-2022-12-15/28840/1

@MichaelOrlov MichaelOrlov mentioned this pull request Feb 1, 2023
Closed
@emersonknapp emersonknapp mentioned this pull request Feb 3, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@amacneil amacneil amacneil left review comments

@emersonknapp emersonknapp emersonknapp approved these changes

@hidmic hidmic Awaiting requested review from hidmic hidmic is a code owner automatically assigned from ros2/tooling-reviewers

@clalancette clalancette Awaiting requested review from clalancette

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Store DB schema versioning in .db3 files
4 participants
@MichaelOrlov @ros-discourse @emersonknapp @amacneil