Import rosbag2_transport Python module on demand #190
Conversation
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
|
LGTM as a short term fix. The better solution in my opinion would be if the Python module wouldn't create and populate a global variable in the module scope but expose a function to be called instead. That would avoid the problem that this issues pops up again when new code imports the module. |
|
@dirk-thomas I believe either of the two are short-term fixes. Importing a C extension shouldn't require such measures nor care. And it actually does not everywhere else but on Windows. That's the true limitation that has to be removed IMHO. |
|
CI (up to
|
There was a problem hiding this comment.
As discussed offline, I am okay with that patch.
However, I'd demand some documentation or pointers for me to re-iterate over this once we found a decent solution in the future. As for now, I understand the problem, but I don't have enough insights to come up with a qualified feedback on this and therefore trust on your opinions.
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
|
@Karsten1987 see 3b2c58a. I'm also testing this on a Windows VM, reproducing the situation in which it first showed itself. I'll keep you posted. |
|
Alright, DLL issues are gone! We're good to go. @mjcarroll may I have your blessing? |
Closes #179. This pull request ensure
ros2bagimportsrosbag2_transporton demand, as opposed to doing so at the module level.I purposefully didn't rehash the
rosbag2_transportmodule to enforce delayed imports as there's been some discussion and work around removing this limitation, and hopefully we might at some point in the future (see ros2/rclpy#417, ros2/rclpy#420 and ros2/rclpy#422).