Skip to content
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.

Sign up for GitHub

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

Jump to bottom

PR: Use control channel for comms #16890

Merged
merged 20 commits into from
Jun 28, 2022
Merged

PR: Use control channel for comms #16890

merged 20 commits into from
Jun 28, 2022

Conversation

impact27
Copy link
Contributor

@impact27 impact27 commented Nov 24, 2021 •
edited by ccordoba12
Loading

Description of Changes

Issue(s) Resolved

Fixes #14894

Affirmation

By submitting this Pull Request or typing my (user)name below,
I affirm the Developer Certificate of Origin
with respect to all commits and content included in this PR,
and understand I am releasing the same under Spyder's MIT (Expat) license.

I certify the above statement is true and correct:

@pep8speaks
Copy link

pep8speaks commented Nov 24, 2021 •
edited
Loading

Hello @impact27! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2022-06-28 17:13:42 UTC

@impact27
comm control
a3308ee
@impact27
git subrepo clone --branch=comm_control --force https://github.com/im…
648c313 
…pact27/spyder-kernels.git external-deps/spyder-kernels

subrepo:
  subdir:   "external-deps/spyder-kernels"
  merged:   "ea9c2930a0"
upstream:
  origin:   "https://github.com/impact27/spyder-kernels.git"
  branch:   "comm_control"
  commit:   "ea9c2930a0"
git-subrepo:
  version:  "0.4.3"
  origin:   "???"
  commit:   "???"
@impact27 impact27 mentioned this pull request Dec 18, 2021
Closed
@impact27 impact27 changed the base branch from 5.x to master June 7, 2022 06:55
Quentin Peter added 3 commits June 7, 2022 08:59
Merge remote-tracking branch 'upstream/master' into comm_control
3282f07
git subrepo clone (merge) --branch=comm_control --force https://githu…
cb9211b 
…b.com/impact27/spyder-kernels.git external-deps/spyder-kernels

subrepo:
  subdir:   "external-deps/spyder-kernels"
  merged:   "025f597c8"
upstream:
  origin:   "https://github.com/impact27/spyder-kernels.git"
  branch:   "comm_control"
  commit:   "025f597c8"
git-subrepo:
  version:  "0.4.1"
  origin:   "https://github.com/ingydotnet/git-subrepo"
  commit:   "a04d8c2"
Merge remote-tracking branch 'upstream/master' into comm_control
ae21acc
@impact27 impact27 mentioned this pull request Jun 12, 2022
Merged
@impact27
Copy link
Contributor Author

@ccordoba12 from qtpy.QtWebEngineWidgets import WEBENGINE doesn't work with qtpy 2.2.0.DEV0

Quentin Peter added 2 commits June 12, 2022 12:04
git subrepo clone (merge) --branch=comm_control --force https://githu…
404bc21 
…b.com/impact27/spyder-kernels.git external-deps/spyder-kernels

subrepo:
  subdir:   "external-deps/spyder-kernels"
  merged:   "c2f7bebd1"
upstream:
  origin:   "https://github.com/impact27/spyder-kernels.git"
  branch:   "comm_control"
  commit:   "c2f7bebd1"
git-subrepo:
  version:  "0.4.1"
  origin:   "https://github.com/ingydotnet/git-subrepo"
  commit:   "a04d8c2"

git subrepo clone --branch=comm_control --force https://github.com/impact27/spyder-kernels.git external-deps/spyder-kernels

subrepo:
  subdir:   "external-deps/spyder-kernels"
  merged:   "2ba517c1b"
upstream:
  origin:   "https://github.com/impact27/spyder-kernels.git"
  branch:   "comm_control"
  commit:   "2ba517c1b"
git-subrepo:
  version:  "0.4.1"
  origin:   "https://github.com/ingydotnet/git-subrepo"
  commit:   "a04d8c2"

git subrepo clone --branch=comm_control --force https://github.com/impact27/spyder-kernels.git external-deps/spyder-kernels

subrepo:
  subdir:   "external-deps/spyder-kernels"
  merged:   "8503fba55"
upstream:
  origin:   "https://github.com/impact27/spyder-kernels.git"
  branch:   "comm_control"
  commit:   "8503fba55"
git-subrepo:
  version:  "0.4.1"
  origin:   "https://github.com/ingydotnet/git-subrepo"
  commit:   "a04d8c2"
fix pep8
5eaa32d
@impact27
Copy link
Contributor Author

The failed tests may not be related to this PR, see https://github.com/spyder-ide/spyder/pull/18256/checks

@ccordoba12
Copy link
Member

from qtpy.QtWebEngineWidgets import WEBENGINE doesn't work with qtpy 2.2.0.DEV0

Ok, that's a serious regression. @dalthviz, could you take a look at that?

The failed tests may not be related to this PR

Yeah, @dalthviz is trying to fix them in PR #18106.

@ccordoba12 ccordoba12 added this to the v6.0alpha1 milestone Jun 12, 2022
ccordoba12
ccordoba12 reviewed Jun 12, 2022
View reviewed changes
Copy link
Member

@ccordoba12 ccordoba12 left a comment

Choose a reason for hiding this comment

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

I left a couple of tiny suggestions, otherwise looks good to me.

spyder/plugins/ipythonconsole/utils/manager.py Outdated Show resolved Hide resolved
spyder/plugins/ipythonconsole/utils/manager.py Outdated Show resolved Hide resolved
impact27 and others added 7 commits June 12, 2022 18:05
@impact27 @ccordoba12
Apply suggestions from code review
ab13d18 
Co-authored-by: Carlos Cordoba <ccordoba12@gmail.com>
remove ssh_parameters
14f7755
forward control port as well
ab478cf
Merge remote-tracking branch 'upstream/master' into comm_control
8dcd372
Merge remote-tracking branch 'upstream/master' into comm_control
41a0296
Merge remote-tracking branch 'upstream/master' into comm_control
784609c
git subrepo clone (merge) --branch=comm_control --force https://githu…
24cda9e 
…b.com/impact27/spyder-kernels.git external-deps/spyder-kernels

subrepo:
  subdir:   "external-deps/spyder-kernels"
  merged:   "04e37121a"
upstream:
  origin:   "https://github.com/impact27/spyder-kernels.git"
  branch:   "comm_control"
  commit:   "04e37121a"
git-subrepo:
  version:  "0.4.1"
  origin:   "https://github.com/ingydotnet/git-subrepo"
  commit:   "a04d8c2"
@impact27 impact27 force-pushed the comm_control branch 2 times, most recently from eaa7282 to 69b9baf Compare June 23, 2022 12:13
@impact27 impact27 mentioned this pull request Jun 28, 2022
Closed
@ccordoba12
Copy link
Member

@impact27, please resync our subrepo with its latest master so we can merge this one.

Quentin Peter added 2 commits June 28, 2022 17:39
Merge remote-tracking branch 'upstream/master' into comm_control
7c9c5b8
git subrepo clone (merge) --branch=master --force https://github.com/…
05e3e6c 
…spyder-ide/spyder-kernels.git external-deps/spyder-kernels

subrepo:
  subdir:   "external-deps/spyder-kernels"
  merged:   "8a161f6a1"
upstream:
  origin:   "https://github.com/spyder-ide/spyder-kernels.git"
  branch:   "master"
  commit:   "8a161f6a1"
git-subrepo:
  version:  "0.4.1"
  origin:   "https://github.com/ingydotnet/git-subrepo"
  commit:   "a04d8c2"
ccordoba12
ccordoba12 reviewed Jun 28, 2022
View reviewed changes
Copy link
Member

@ccordoba12 ccordoba12 left a comment

Choose a reason for hiding this comment

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

@impact27, a couple of small suggestions for you, then this should be ready.

spyder/plugins/ipythonconsole/utils/manager.py Outdated Show resolved Hide resolved
spyder/plugins/ipythonconsole/utils/manager.py Outdated Show resolved Hide resolved
impact27 and others added 2 commits June 28, 2022 19:07
@impact27 @ccordoba12
Apply suggestions from code review
3a91126 
Co-authored-by: Carlos Cordoba <ccordoba12@gmail.com>
move SpyderKernelClient
031fa64
ccordoba12
ccordoba12 approved these changes Jun 28, 2022
View reviewed changes
Copy link
Member

@ccordoba12 ccordoba12 left a comment

Choose a reason for hiding this comment

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

Looks good to me, thanks @impact27!

@ccordoba12 ccordoba12 merged commit 640940b into spyder-ide:master Jun 28, 2022
@alandema alandema mentioned this pull request Jul 14, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ccordoba12 ccordoba12 ccordoba12 approved these changes

Assignees

@impact27 impact27

Labels
component:IPython Console
Projects
None yet
Milestone
v6.0alpha1
Development

Successfully merging this pull request may close these issues.

Variable explorer doesn't show variables on remote kernel when using localhost forwarded ports
4 participants
@impact27 @pep8speaks @ccordoba12 @dalthviz