Skip to content

Handle failures with conda activation#4669

Merged
rchiodo merged 4 commits into
masterfrom
rchiodo/handle_conda_failure
Mar 7, 2019
Merged

Handle failures with conda activation#4669
rchiodo merged 4 commits into
masterfrom
rchiodo/handle_conda_failure

Conversation

@rchiodo

@rchiodo rchiodo commented Mar 7, 2019

Copy link
Copy Markdown

For #4424

  • Pull request represents a single change (i.e. not fixing disparate/unrelated things in a single PR)
  • Title summarizes what is changing
  • Has a news entry file (remember to thank yourself!)
  • Has sufficient logging.
  • Has telemetry for enhancements.
  • Unit tests & system/integration tests are added/updated
  • Test plan is updated as appropriate
  • package-lock.json has been regenerated by running npm install (if dependencies have changed)
  • The wiki is updated with any design decisions/details.

@rchiodo rchiodo self-assigned this Mar 7, 2019
return this.parseEnvironmentOutput(result.stdout);
} catch (e) {
// Some callers want this to bubble out, others don't
if (allowExceptions) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The trace seems useful in either case I'd move it outside the else.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Sounds good.


In reply to: 263516541 [](ancestors = 263516541)

Comment thread src/client/datascience/history.ts Outdated
}
}

return usableInterpreter ? true: false;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

usableInterpreter === undefined now doesn't throw an Error. I think we would still want one in that case.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

You're right. I changed that behavior. Will fix.


In reply to: 263517413 [](ancestors = 263517413)

@codecov

codecov Bot commented Mar 7, 2019

Copy link
Copy Markdown

Codecov Report

Merging #4669 into master will decrease coverage by 1%.
The diff coverage is 71%.

@@           Coverage Diff           @@
##           master   #4669    +/-   ##
=======================================
- Coverage      77%     77%   -<1%     
=======================================
  Files         446     447     +1     
  Lines       21218   21490   +272     
  Branches     3471    3539    +68     
=======================================
+ Hits        16293   16425   +132     
- Misses       4921    5058   +137     
- Partials        4       7     +3
Flag Coverage Δ
#Linux 66% <35%> (ø) ⬇️
#Windows 66% <35%> (ø) ⬇️
#macOS 66% <35%> (ø) ⬇️

@IanMatthewHuff IanMatthewHuff left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

:shipit:

@rchiodo rchiodo merged commit 1db75b5 into master Mar 7, 2019
@rchiodo rchiodo deleted the rchiodo/handle_conda_failure branch March 7, 2019 22:56
@lock lock Bot locked as resolved and limited conversation to collaborators Jul 30, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants