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

Add checks for querying remote end points in case of network errors. #429

Merged
merged 1 commit into from
Sep 27, 2024

Conversation

tfoote
Copy link
Member

@tfoote tfoote commented Sep 26, 2024

No description provided.

@tfoote
Copy link
Member Author

tfoote commented Sep 26, 2024

@rkent if you could confirm this works for you that would be great. I ran into it when running locally when the network config changed while this was running in the background.

@rkent
Copy link

rkent commented Sep 27, 2024

Trying to figure out if "this works for you" I was trying to understand why I was not seeing the Continuous Integration text in the UI. Turns out this is a regressions from my #417. package_qna_updates.html was not using prettify.css and prettify.js (so I deleted their reference and files). Turns out that prettify.css IS used in package_instance.html for the CI stuff. It got moved into package_qna_updates.html at some point, but that is included in the package_instance.html where the CSS is actually needed.

So I'm quite sure how to interpret "this works for you" since it doesn't, but not the fault of this PR. I think I'll fix the prettify.css issues, then try the current PR on top of that.

@tfoote
Copy link
Member Author

tfoote commented Sep 27, 2024

I was only fixing the build crashing, not checking the output rendering. Without these catches, I wasn't able to get through the scanning process on my dev machine due to network changes happening periodically on my host. So if the build passes I'd consider this to work for you.

Copy link

@rkent rkent left a comment

Choose a reason for hiding this comment

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

LGTM.

I simulated the error by adding a fake docs.ros.org to etc/hosts and it continued cleanly, generating the new error messages.

@tfoote tfoote merged commit a4fbf9f into ros2 Sep 27, 2024
2 checks passed
@tfoote tfoote deleted the network_disconnect_robustness branch September 27, 2024 17:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants