fix(dependency_walker): don't get dependencies of non ROS packages

[Guillaumebeuzeboc]

When using the DependencyWalker and calling get_recursive_depends with the ros_packages_only to False, the DependencyWalker tries to get the dependencies of every dependency (even the non-ROS ones).
In order to get the dependencies of a package we must get the corresponding ROS package (for the XML etc). This is not possible for non ROS packages (like CMake, boost etc). Hence this causes a KeyError.

I simply added a check to verify if the package is a ROS package or not before trying to get its dependencies.

[artivis]

Hi @cottsay, could we get a workflow approval? Thanks.

[tfoote]

Sorry for the long delay. This looks like a good change. I'm not sure why the Action workflows aren't triggering. Can you try to rebase this to trigger it?

[Guillaumebeuzeboc]

@tfoote , rebased and pushed the branch. It's now waiting for a "workflow approval"

[tfoote]

Approved the workflow it's running now

[codecov-commenter]

Codecov Report

❌ Patch coverage is 0% with 2 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (master@9e67766). Learn more about missing BASE report.

Files with missing lines	Patch %	Lines
src/rosdistro/dependency_walker.py	0.00%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff            @@
##             master     #171   +/-   ##
=========================================
  Coverage          ?   39.79%           
=========================================
  Files             ?       51           
  Lines             ?     3257           
  Branches          ?      660           
=========================================
  Hits              ?     1296           
  Misses            ?     1785           
  Partials          ?      176           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[Guillaumebeuzeboc]

It seems that one test had a timeout and cancelled the remaining workflows. It looks like I cannot retry it myself.

[tfoote]

@cottsay merged in the CI fix and the workflows are running now.

One question for this is that calling this with a non-ROS package seems like an error. Should we make it respond as an error rather than silently return an empty set?

[Guillaumebeuzeboc]

@cottsay merged in the CI fix and the workflows are running now.

One question for this is that calling this with a non-ROS package seems like an error. Should we make it respond as an error rather than silently return an empty set?

It has been 2 years, so I honestly do not remember the use case I was covering. It sounds reasonable to respond as an error.
Made the change.

[tfoote]

Thanks for the patience on this and the follow up.

lgtm and I triggered CI successfully