ros2component fix to Dashing?

[pbaughman]

Hey guys,

We're on 'Dashing' but our CI is blowing up a lot with an issue that I believe was fixed in PR #322 Would you guys consider cherry-picking this fix to the 'dashing' branch so we can get it without having to take the other ~35 commits in between? It looks like it cherry-picks cleanly, and the tests pass on the nightly docker.

Thanks! Feel free to close this issue if the answer is 'no'

[jacobperron]

#322 depends on ros2/rclpy#413, which depends on ros2/rcl#492.

ros2/rcl#492 adds a macro and changes the possible return value for several functions.
I believe everything is API/ABI compatible, so I think we could backport the three PRs in question.

[jacobperron]

I've started looking at doing the backport, but it's a bit larger than anticipated. The necessary changes actually touch the RMW layer:

• Add specific return type for non existent node rmw#182
• Return specific error code when node is not found rmw_fastrtps#311
• Return specific error when not finding a node name rmw_connext#365
• Return specific error when not finding a node name rmw_opensplice#281

And there are some conflicts with another addition to RMW ros2/rmw#179.
The conflicts are not that difficult to resolve, but if I go forward opening a series of backport PRs I think they deserve close attention so we don't accidentally break things.

Possibly a less risky approach would be to backport an earlier iteration of the fix (bd6cef2), where we catch a generic RuntimeError instead of the new NodeNameNonExistentError that was introduced in the PRs I've listed above.

@hidmic @dirk-thomas @ivanpauno What do you guys think?
I don't mind doing the more invasive option, backporting the new error code, if you think it's acceptable.

[dirk-thomas]

I don't think backporting this change is worth that amount of work and potential for regression. I am ok with implementing the previous hack / wildcard-exception-catch on the Dashing branch.

[ivanpauno]

Yes, I was going to start backporting this until I realize that the amount of dependencies were huge.
The complete list also includes ros2/rcl#499 ros2/rcl#498.

Also, I don't think that adding a new return code and a new exception is API compatible. IMO, it breaks API.

Implementing the wildcard-exception-catch on Dashing branch sounds like a good idea to me.

[jacobperron]

Thanks for your feedback, I've opened up a backport for the wildcard-exception-catch: #342