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

fix: reworked HTTP status code handling for 2xx and 3xx ranges (MWTELE-68, MWTELE-69) #83

Merged
merged 1 commit into from
May 11, 2023

Conversation

jponge
Copy link
Collaborator

@jponge jponge commented May 2, 2023

This ensures any 2xx response is considered valid. The client now follows redirects (3xx range).

JIRA: https://issues.redhat.com/browse/MWTELE-68
JIRA: https://issues.redhat.com/browse/MWTELE-69

@jponge jponge requested review from ehsavoie and kittylyst May 2, 2023 16:23
@jponge
Copy link
Collaborator Author

jponge commented May 2, 2023

/cc @mocenas

…E-68, MWTELE-69)

This ensures any 2xx response is considered valid.
The client now follows redirects (3xx range).

JIRA: https://issues.redhat.com/browse/MWTELE-68
JIRA: https://issues.redhat.com/browse/MWTELE-69
@jponge jponge force-pushed the bug/MWTELE-68-and-MWTELE-69 branch from db9b57c to f58c195 Compare May 2, 2023 17:18
ERROR_HTTP_SEND_INVALID_CONTENT_TYPE,
"Content type of payload is unsupported");
}
case 5:
Copy link
Collaborator Author

@jponge jponge May 2, 2023

Choose a reason for hiding this comment

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

Note: I have deliberately kept multiple case label statements based on the original OpenAPI spec of the Go insights backend. These are not very clean switch constructs, but I wanted to keep track of the expected status codes.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Why not defining an enum for each 'category': could be more readable WDYT ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Shouldn't we deal with 403 as maybe the subscription is no longer active or something like this ?

@mocenas
Copy link
Contributor

mocenas commented May 11, 2023

LGTM

@jponge jponge merged commit 4ac9047 into main May 11, 2023
@jponge jponge deleted the bug/MWTELE-68-and-MWTELE-69 branch May 11, 2023 09:36
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