Skip to content
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

[parsing] Plumb downloader errors through to the exception text #19081

Merged

Conversation

jwnimmer-tri
Copy link
Collaborator

@jwnimmer-tri jwnimmer-tri commented Mar 28, 2023

Closes #19047. Towards #9498.


This change is Reviewable

Also take this opportunity to update the tests to prefer pathlib.Path
uniformly (vs os.path).
@jwnimmer-tri jwnimmer-tri marked this pull request as ready for review March 28, 2023 19:40
@jwnimmer-tri jwnimmer-tri added priority: low release notes: none This pull request should not be mentioned in the release notes priority: backlog and removed priority: low labels Mar 28, 2023
@jwnimmer-tri
Copy link
Collaborator Author

+@rpoyner-tri for feature review, please (at some point). This is very low priority.

Copy link
Contributor

@rpoyner-tri rpoyner-tri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

Reviewed 5 of 5 files at r1, all commit messages.
Reviewable status: needs at least two assigned reviewers (waiting on @jwnimmer-tri)

@jwnimmer-tri
Copy link
Collaborator Author

+@sammy-tri for platform review, please. This is very low priority.

Copy link
Contributor

@sammy-tri sammy-tri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

Reviewed 5 of 5 files at r1, all commit messages.
Reviewable status: :shipit: complete! all discussions resolved, LGTM from assignees rpoyner-tri(platform),sammy-tri(platform) (waiting on @jwnimmer-tri)

@sammy-tri sammy-tri merged commit 564dfb9 into RobotLocomotion:master Mar 30, 2023
@jwnimmer-tri jwnimmer-tri deleted the package-map-error-logging branch March 30, 2023 21:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: backlog release notes: none This pull request should not be mentioned in the release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[parsing] Better error logging from package_downloader
3 participants