Skip to content
This repository was archived by the owner on Mar 13, 2022. It is now read-only.

Refactor stream package to enable common method helpers for other streaming api classes. - #211

Merged
k8s-ci-robot merged 2 commits into
kubernetes-client:masterfrom
iciclespider:refactor-for-port-forward
Aug 28, 2020
Merged

Refactor stream package to enable common method helpers for other streaming api classes.#211
k8s-ci-robot merged 2 commits into
kubernetes-client:masterfrom
iciclespider:refactor-for-port-forward

Conversation

@iciclespider

@iciclespider iciclespider commented Aug 27, 2020

Copy link
Copy Markdown
Contributor

This puts in place the methods needed by this port forwarding implementation: #210
Also, simplify the patten used to monkey patch the ApiClient.request method.

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Aug 27, 2020
@k8s-ci-robot k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Aug 27, 2020
@iciclespider

iciclespider commented Aug 27, 2020

Copy link
Copy Markdown
Contributor Author

/assign @yliaog

@iciclespider iciclespider mentioned this pull request Aug 27, 2020
Merged
@iciclespider
iciclespider force-pushed the refactor-for-port-forward branch from 7ea9914 to fd62214 Compare August 27, 2020 04:28
yliaog
yliaog reviewed Aug 28, 2020
View reviewed changes
Comment thread stream/ws_client.py Outdated

def websocket_call(api_client, _method, url, **kwargs):
"""An internal function to be called in api-client when a websocket
connection is required. args and kwargs are the parameters of

@yliaog yliaog Aug 28, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

the comment needs to be updated

Comment thread stream/stream.py Outdated
@iciclespider
Put extracting the "configuration" back into the stream.py module, an…
a00ed7f 
…d use

functools.partial to orchestrate calling the websocket request hanlder.
@iciclespider
iciclespider force-pushed the refactor-for-port-forward branch from 6d2cc42 to a00ed7f Compare August 28, 2020 02:10
@codecov-commenter

codecov-commenter commented Aug 28, 2020

Copy link
Copy Markdown

Codecov Report

Merging #211 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #211   +/-   ##
=======================================
  Coverage   92.37%   92.37%           
=======================================
  Files          13       13           
  Lines        1613     1613           
=======================================
  Hits         1490     1490           
  Misses        123      123

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 54d188f...a00ed7f. Read the comment docs.

@yliaog

yliaog commented Aug 28, 2020

Copy link
Copy Markdown
Contributor

thanks for the pr
/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Aug 28, 2020
@k8s-ci-robot

k8s-ci-robot commented Aug 28, 2020

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: iciclespider, yliaog

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 28, 2020
@k8s-ci-robot
k8s-ci-robot merged commit 471a678 into kubernetes-client:master Aug 28, 2020
This was referenced Mar 15, 2021
Closed
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@mbohlool mbohlool Awaiting requested review from mbohlool
1 more reviewer
@yliaog yliaog yliaog left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

@yliaog yliaog

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm Indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@iciclespider @codecov-commenter @yliaog @k8s-ci-robot