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

Create new error.connection.registry to avoid internal errors being thrown for registry connection issues #1091

Merged
merged 2 commits into from
Jan 6, 2025

Conversation

al-niessner
Copy link
Contributor

🗒️ Summary

Added new ProblemType.CONNECTION_ERROR and changed internal error to this.

⚙️ Test Data and/or Report

All unit tests below pass and visual inspection of code changes.

♻️ Related Issues

Closes #1085

@al-niessner al-niessner requested a review from a team as a code owner December 18, 2024 23:52
@@ -44,6 +44,8 @@ public enum ProblemType {

CONTEXT_REFERENCE_NOT_FOUND("error.label.context_ref_not_found"),

CONNECTION_ERROR("error.connection.registry"),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Make the characters inside quotes anything you want.

Copy link
Member

@nutjob4life nutjob4life left a comment

Choose a reason for hiding this comment

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

✅ Visual inspection looks good

✅ Testing looks good

WARNING] Tests run: 252, Failures: 0, Errors: 0, Skipped: 23
[INFO] 
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  09:37 min

Approved!

Unnecessary import
@al-niessner al-niessner self-assigned this Jan 6, 2025
@al-niessner al-niessner merged commit 8b383e4 into main Jan 6, 2025
4 checks passed
@al-niessner al-niessner deleted the issue_1085 branch January 6, 2025 19:05
@jordanpadams
Copy link
Member

@al-niessner @nutjob4life For our PR reviews, can we also include reviews of the titles of the PRs to be end-user readable? Those are used to generate the release notes automatically. Unfortunately, however GitHub does it you can't update them after the PR has been merged (maybe merge commits?)

@jordanpadams jordanpadams changed the title 1085: move from internal error to made up error Create new error.connection.registry to avoid internal errors being thrown for registry connection issues Jan 6, 2025
@nutjob4life
Copy link
Member

@jordanpadams I thought the release notes were generated from issues, not pull requests. Is it both?

@jordanpadams
Copy link
Member

@nutjob4life the CHANGELOG is generated using issues, GitHub release notes are generated using PR titles: https://docs.github.com/en/repositories/releasing-projects-on-github/automatically-generated-release-notes . The intent being that CHANGELOGs are more for developers/owners of the software, versus release notes are intended for the end user to know what was actually done. We can manually update the release notes that GitHub generates, so it isn't the end of the world, it just helps @tloubrieu-jpl and I when we need to tag a release

@al-niessner
Copy link
Contributor Author

@jordanpadams - that is 100% subjective judgement call which means you are signing up to all PR merges. Sidebar: you can modify the script that does it; meaning write your own then replace it and remove #: on all titles.

@nutjob4life
Copy link
Member

@jordanpadams thanks!

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.

Improve error handling for Registry connection issues
3 participants