gaurav singh has posted comments on this change. ( http://gerrit.cloudera.org:8080/22424 )
Change subject: IMPALA-13675: OAuth AuthN Support for Impala Shell ...................................................................... Patch Set 6: (4 comments) http://gerrit.cloudera.org:8080/#/c/22424/4/shell/impala_shell.py File shell/impala_shell.py: http://gerrit.cloudera.org:8080/#/c/22424/4/shell/impala_shell.py@1008 PS4, Line 1008: try: > Does an HTTPException get thrown for error codes other than 100/200/300? It should only cover timeouts or connection related error condition. Added an extra if block to check for 200 status. http://gerrit.cloudera.org:8080/#/c/22424/6/shell/impala_shell.py File shell/impala_shell.py: http://gerrit.cloudera.org:8080/#/c/22424/6/shell/impala_shell.py@1013 PS6, Line 1013: "application/json > Wouldn't hurt to add a charset=utf-8 onto this header value: https://devel Done http://gerrit.cloudera.org:8080/#/c/22424/6/shell/impala_shell.py@1014 PS6, Line 1014: conn.request("POST", self.oauth_endpoint, payload.encode('utf-8'), headers) > Does the '.request' function automatically handle HTTP 301/302 redirects? The 301,302 redirect is done by the server and the client should follow the http protocol. I am not sure if 301/302 redirect is involved in this flow. http://gerrit.cloudera.org:8080/#/c/22424/6/shell/impala_shell.py@1024 PS6, Line 1024: conn.close() > What happens here if the 'http.client.HTTPSConnection' function throws an e Done. Added an if check before close. -- To view, visit http://gerrit.cloudera.org:8080/22424 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I84e26d54f6a53696660728efb239ffd43de4c55d Gerrit-Change-Number: 22424 Gerrit-PatchSet: 6 Gerrit-Owner: gaurav singh <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Jason Fehr <[email protected]> Gerrit-Reviewer: gaurav singh <[email protected]> Gerrit-Comment-Date: Thu, 13 Mar 2025 17:45:58 +0000 Gerrit-HasComments: Yes
