Skip to content

Reapply "Implementation for /exec using websocket" #410

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

Closed
wants to merge 1 commit into from

Conversation

dulek
Copy link

@dulek dulek commented Dec 5, 2017

This commit reapplies PR #120 that was removed by #353. Below is the
original commit message.

inspired by the POC from @chekolyn

  • Adds a new requirement on websocket-client
  • Add a new class WSClient that uses WebSocketApp from
    the websocket-client.
  • Make sure we pass Authorization header
  • Make sure we honor the SSL settings in configuration
  • Some of the code will get overwritten when we generate
    fresh classes from swagger definition. To remind us
    added a e2e test so we don't lose the changes
  • Added a new configuration option to enable/disable failures
    when hostnames in certificates don't match

Fixes #58
Fixes #409

This commit reapplies PR kubernetes-client#120 that was removed by kubernetes-client#353. Below is the
original commit message.

inspired by the POC from @chekolyn

* Adds a new requirement on websocket-client
* Add a new class WSClient that uses WebSocketApp from
  the websocket-client.
* Make sure we pass Authorization header
* Make sure we honor the SSL settings in configuration
* Some of the code will get overwritten when we generate
  fresh classes from swagger definition. To remind us
  added a e2e test so we don't lose the changes
* Added a new configuration option to enable/disable failures
  when hostnames in certificates don't match

Fixes kubernetes-client#58
Fixes kubernetes-client#409
@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Dec 5, 2017
@dims
Copy link
Collaborator

dims commented Dec 13, 2017

@dulek the websocket related code is still there ... under https://github.com/kubernetes-client/python-base/blob/master/stream/ws_client.py

@dulek
Copy link
Author

dulek commented Dec 13, 2017

@dims: Oh, so I assume only change to api_client.py is needed?

@mbohlool
Copy link
Contributor

mbohlool commented Dec 13, 2017

@dulek api_client.py is an auto-generated file and we should not edit it. Please refer to example here for how to do exec calls in the new python client world :)

@mbohlool mbohlool closed this Dec 13, 2017
@dulek
Copy link
Author

dulek commented Dec 13, 2017

Oh. Thanks for pointing it out and sorry for all the fuss!

@mbohlool
Copy link
Contributor

@dulek You are welcome and no worries. Let me know how your experience went with exec calls. Any improvements to that process would be most welcome.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants