Jason Fehr has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/22424 )

Change subject: IMPALA-13675: OAuth AuthN Support for Impala Shell
......................................................................


Patch Set 8:

(7 comments)

http://gerrit.cloudera.org:8080/#/c/22424/8//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/22424/8//COMMIT_MSG@15
PS8, Line 15: Testing(okta oauth server):
Is there any way to add automated tests possibly using a mock for the auth 
server?  That would allow for asserting the request payload contents and 
asserting successes/errors are properly handled.


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:
> It should only cover timeouts or connection related error condition. Added
Done


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
> Done
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)
> The 301,302 redirect is done by the server and the client should follow the
Done


http://gerrit.cloudera.org:8080/#/c/22424/6/shell/impala_shell.py@1024
PS6, Line 1024:       except Exception as e:
> Done. Added an if check before close.
Done


http://gerrit.cloudera.org:8080/#/c/22424/8/shell/impala_shell.py
File shell/impala_shell.py:

http://gerrit.cloudera.org:8080/#/c/22424/8/shell/impala_shell.py@1017
PS8, Line 1017:           print("HTTP error: ", res.status)
Should we print out the response body here too in case there is more 
information about the response status?


http://gerrit.cloudera.org:8080/#/c/22424/8/shell/impala_shell.py@1021
PS8, Line 1021:         self.oauth = json_body["access_token"]
Will the refresh token flow also be implemented?



--
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: 8
Gerrit-Owner: gaurav singh <gsi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: gaurav singh <gsi...@cloudera.com>
Gerrit-Comment-Date: Fri, 14 Mar 2025 16:08:07 +0000
Gerrit-HasComments: Yes

Reply via email to