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

containers: Catch decode_json() exception in engine->info() #20878

Merged
merged 1 commit into from
Dec 30, 2024

Conversation

ricardobranco777
Copy link
Contributor

@ricardobranco777 ricardobranco777 commented Dec 30, 2024

The current code doesn't catch JSON exceptions and the content of stderr is never shown.

Copy link
Contributor

@rbmarliere rbmarliere left a comment

Choose a reason for hiding this comment

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

LGTM

@ricardobranco777 ricardobranco777 changed the title containers: Fix issue with failing decode_json() in engine->info() containers: Fix handling of decode_json() exception in engine->info() Dec 30, 2024
Copy link
Member

@asmorodskyi asmorodskyi left a comment

Choose a reason for hiding this comment

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

commit message/ PR message a bit misleading . unless I have missed something you didn't "fix" anything . You add more verbosity to the error in case of decode_json error. Can you please tweak commit/PR message to something like "Add more verbosity to decode_json errors" ? other than that LGTM

@ricardobranco777 ricardobranco777 changed the title containers: Fix handling of decode_json() exception in engine->info() containers: Catch decode_json() exception in engine->info() Dec 30, 2024
@asmorodskyi asmorodskyi merged commit b1787c2 into os-autoinst:master Dec 30, 2024
10 checks passed
@ricardobranco777 ricardobranco777 deleted the json branch December 30, 2024 15:42
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.

3 participants