-
Notifications
You must be signed in to change notification settings - Fork 48
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
HTTPS support #27
HTTPS support #27
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, let me enable HTTPS on one of my environments and give it a try later this week.
@lresende any success? |
e6b4d68
to
e7dd511
Compare
@lresende since my refactoring PR got merged, I recreated this branch to make it compatible. |
@@ -19,13 +19,15 @@ class ApplicationMaster(BaseYarnAPI): | |||
:param int port: Proxy HTTP port | |||
:param int timeout: API connection timeout in seconds | |||
:param boolean kerberos_enabled: Flag identifying is Kerberos Security has been enabled for YARN | |||
:param boolean https: Flag identifying HTTPS is used instead of HTTP for YARN | |||
:param boolean https_verify: Flag identifying SSL certificate should be ignored in case of HTTPS |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As we are forwarding this parameter all way to requests
, could you please use/append similar documentation as the one available in requests: Either a boolean, in which case it controls whether we verify the server’s TLS certificate, or a string, in which case it must be a path to a CA bundle to use
@dimon222 Could you please rebase this as well. |
Closed, better approach is suggested there - #33 |
Fix for #19
The only thing is - do we need to apply any corrections in hadoop_conf detection to pull HTTPS urls out as well, or no?
PS: this will conflict with #26, might require rebase incase of merge needs