Sanitize bagfile splitting CLI input #226
Conversation
Signed-off-by: Prajakta Gokhale <prajaktg@amazon.com>
Signed-off-by: Prajakta Gokhale <prajaktg@amazon.com>
|
@Karsten1987 please take a look. Thanks! |
| @@ -37,6 +37,8 @@ class ROSBAG2_STORAGE_PUBLIC ReadWriteInterface | |||
| uint64_t get_bagfile_size() const override = 0; | |||
|
|
|||
| std::string get_storage_identifier() const override = 0; | |||
|
|
|||
| virtual uint64_t get_minimum_split_file_size() const = 0; | |||
There was a problem hiding this comment.
I personally think we should have a more general function such as validate_storage_options() which allows to validate all possible storage options. That makes it a bit easier to extend the functionality later on if more options are added in the future wo/ adding a new function to the interface for each of them.
There was a problem hiding this comment.
I believe that would be a responsibility of writer and not the individual storage_interfaces. It can be extended by adding new validation if new storage_options get added in future, with corresponding changes if needed in storage_interfaces. @Karsten1987 what do you think?
There was a problem hiding this comment.
I am okay with this for now in order to push this feature forward. But I think we should address this more generally latest when different storage options come into the game, such as splitting by time or such.
|
@ros2/aws-oncall - please run this CI job |
|
As per discussion on #212, moving bagfile splitting CLI input validation to
SequentialWriterandstorage_interfaces.Closes https://github.com/ros-security/aws-roadmap/issues/6.
Signed-off-by: Prajakta Gokhale prajaktg@amazon.com