-
Notifications
You must be signed in to change notification settings - Fork 74
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
mock launch components causing rosdoc2 hang #425
base: rolling
Are you sure you want to change the base?
Conversation
Signed-off-by: R. Kent James <[email protected]>
This is now ready for review, I fixed the linting issues. Just be clear what this is about. The existing launch_ros documentation in rosdoc2 does not show the python API. So if you go to https://docs.ros.org/en/rolling/p/launch_ros/launch_ros.actions.composable_node_container.html the documentation is empty. I have a run of rosdoc2 available to view with this PR applied, try https://prrosdoc2.rosdabbler.com/rolling/launch_ros/ Looking at that composable_node_container at https://prrosdoc2.rosdabbler.com/rolling/launch_ros/launch_ros.actions.composable_node_container.html you can see the python API documentation being shown. |
This is great to resolve. Are there upstream issues that we could link from the code with comments such that people can know when/if these workarounds can be removed. Or know why they're there so that they don't get removed and accidentally break the documentation. |
I could not find existing upstream issues related to the problems I am trying to fix. Sphinx had an earlier PR that supposedly supports mocked decorators, not sure why it is not working for us. The other issue, with SomeSubstitutionsType_types_tuple, is a different issue. I think it is because mocked objects to not support addition. With >1200 open issues in Sphinx, and my limited understanding of how all of this is supposed to work in Sphinx, I did not feel like I could contribute a meaningful issue in Sphinx that would be actionable. I could not find existing issues that seemed relevant. At the moment I am not aware of other ros2 packages with similar problems, but finding these requires noting that a package is supposed to have documentation, but it is not actually appearing, which is not easy to automate over almost 2000 packages. I think I started focusing on this repo for an unrelated issue that was causing rosdoc2 to die completely, which turned out to be related to me running Sphinx 8 in my build farm. The solution that this PR proposes essentially preloads the sys.modules cache with the specific mocked object that we want. It therefore is dependent on the implementation of sphinx.modules. I don't like that, it seems fragile and opaque, but I am not sure what else to do. Because this is confined to conf.py and therefore just the documentation, the fragility is not as risky as it would be in the active code of the package. But I am open to any suggestions on how to improve this. |
@tfoote Friendly ping for the follow-up review. |
Fixes #424
There are multiple bug workarounds for upstream packages here.
Within rosdoc2, the supplied code has (relatively minor) workarounds for ros-infrastructure/rosdoc2#156 (hence the supplied additional "Design" document), as well as a complex sys.path in conf.py that will not be needed after ros-infrastructure/rosdoc2#155
But this PR as supplied should work with the existing rosdoc2.