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.

Sign up for GitHub

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

Jump to bottom

Fixes showing server errors on cli sql calls #1720

Merged
merged 1 commit into from
Sep 18, 2024
Merged

Fixes showing server errors on cli sql calls #1720

merged 1 commit into from
Sep 18, 2024

Conversation

mamcx
Copy link
Contributor

@mamcx mamcx commented Sep 18, 2024

Description of Changes

The cli incorrectly tries to parse as json an http error masquing the problem. Now it checks that the StatusCode is within 200-299. before returning.

Expected complexity level and risk

1

Testing

  • Check the sql calls that could generar error 500.
  • *Use my branch of adding a new system table that is broken to get errors.

@mamcx mamcx added CLI only This change only affects the CLI behavior release-1.0 labels Sep 18, 2024
@mamcx mamcx self-assigned this Sep 18, 2024
@mamcx mamcx requested review from jdetter and bfops September 18, 2024 17:03
bfops
bfops approved these changes Sep 18, 2024
View reviewed changes
Copy link
Collaborator

@bfops bfops left a comment

Choose a reason for hiding this comment

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

Seems reasonable enough!

@mamcx mamcx added this pull request to the merge queue Sep 18, 2024
Merged via the queue into master with commit 3179f6d Sep 18, 2024
11 checks passed
mamcx added a commit that referenced this pull request Sep 20, 2024
@mamcx
Fixes showing server errors on cli sql calls (#1720)
4b5cf08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bfops bfops bfops approved these changes

@jdetter jdetter Awaiting requested review from jdetter jdetter is a code owner

Assignees

@mamcx mamcx

Labels
CLI only This change only affects the CLI behavior release-1.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@mamcx @bfops