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

Prompt to select env when there is no Jupyter env #11109

Merged
merged 1 commit into from
Aug 15, 2022
Merged

Conversation

DonJayamanne
Copy link
Contributor

Fixes #1110

@DonJayamanne DonJayamanne requested a review from a team as a code owner August 12, 2022 02:04
@@ -112,6 +115,17 @@ export class JupyterInterpreterService {
// Use current interpreter.
interpreter = await this.interpreterService.getActiveInterpreter(undefined);
if (!interpreter) {
if (err) {
Copy link
Member

Choose a reason for hiding this comment

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

I read this once or twice and honestly it still looks a bit off so I'll just ask. I'm not sure that I understand the flow here. Per my reading of the bug this issue is for when there is a selected Jupyter interpreter, but for when something happens to that Jupyter interpreter which makes it fail to start. So in that case getSelectedInterpreter would still have the selected jupyter interpreter and it wouldn't go down this path at all.

Copy link
Member

Choose a reason for hiding this comment

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

This seem restricted to a very odd / unlikely case where we have an error, but we also don't have a selected Jupyter extension or an active python interpreter.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

here's the work flow,

  • user selects an en
  • deletes the env as per issue
  • now they get a quick pick without any error message

this charge merely displays the error message

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ie workflow is better now, rather than displaying a quick pick for no reason, which would be very weird for the user. now the quick pick is after a user action (button click) from the prompt

Copy link
Member

Choose a reason for hiding this comment

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

I was under the impression that the getActiveInterpreter right above this almost always returns with something unless there is no python detected at all on the system. Is that not the case?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

nope, because the active interpreter happened to be the same interpreter that was used for jupyter

@DonJayamanne DonJayamanne merged commit 67de927 into main Aug 15, 2022
@DonJayamanne DonJayamanne deleted the issue1110 branch August 15, 2022 19:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Jupyter interpreter can be invalid and break things
2 participants