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.

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

Can the ports attributes provider use a selector? #13553

Closed
alexr00 opened this issue May 24, 2023 · 4 comments · Fixed by #15243
Closed

Can the ports attributes provider use a selector? #13553

alexr00 opened this issue May 24, 2023 · 4 comments · Fixed by #15243
Assignees
Labels
debt Code quality issues notebook-execution Kernels issues (start/restart/switch/execution, install ipykernel)
Milestone

Comments

@alexr00
Copy link
Member

alexr00 commented May 24, 2023

This extension registers a ports attributes provider so that ports that are listened on for the internal workings of the extension don't get picked up by automatic port forwarding when connected to a remote. ✅

Right now, the provider is registered with an empty selector, which means that VS Code will ask this extension for port attributes for each and every port that it automatically forwards.

Is it feasible to use a selector when the ports attributes provider is registered here?

this.disposables.push(workspace.registerPortAttributesProvider({}, this));
} catch (ex) {

I'm thinking about having an error if the selector is empty like this. The selector will result in a better experience for users as it will prevent extra trips to the extension host for ports that won't have attributes provided.

@alexr00 alexr00 added the feature-request Request for new features or functionality label May 24, 2023
@DonJayamanne DonJayamanne added this to the Backlog milestone Dec 4, 2023
@DonJayamanne
Copy link
Contributor

DonJayamanne commented Dec 5, 2023

@alexr00
The way this is works is

  • We dynamically generate a few ports,
  • The CLI is also dynamic
  • Then we start a process based on the above CLI and port

Thus the suggestion i have is,

  • Just as we're about to start the new process, I can register port attributes provider using the above ports
  • & when the above process dies, we can dispose the above provider.

I believe this will work, only difference from what we have today is that

  • we will end up dynamically registring/un-registring a few of these as kernels (process) are started and shutdown. Then again, I do not think thats an issue.

Please can you advice this approach works.
thanks

@DonJayamanne DonJayamanne added debt Code quality issues notebook-execution Kernels issues (start/restart/switch/execution, install ipykernel) and removed feature-request Request for new features or functionality labels Dec 5, 2023
@alexr00
Copy link
Member Author

alexr00 commented Dec 5, 2023

@DonJayamanne I don't see a problem with that. If you find that it doesn't work then I will fix whatever bug that is uncovered.

@alexr00
Copy link
Member Author

alexr00 commented Feb 27, 2024

@DonJayamanne thanks for making this change! I will make the change in VS Code Insiders to start logging an error when an empty selector is passed. Then, next iteration I'll start throwing.

@DonJayamanne
Copy link
Contributor

Apologies for not getting to this sooner

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 13, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
debt Code quality issues notebook-execution Kernels issues (start/restart/switch/execution, install ipykernel)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants