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

Ensure terminal cwd exists #755

Merged
merged 6 commits into from Mar 25, 2022
Merged

Conversation

fcollonval
Copy link
Member

Check that the terminal current working directory exists. If it does not test if it is relative to the server root directory otherwise ignore it.

Follow-up of #749

@fcollonval
Copy link
Member Author

Thanks a lot @rccern for #749 - I opened this one to be more robust.

Copy link
Collaborator

@blink1073 blink1073 left a comment

Choose a reason for hiding this comment

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

Thank you!

@blink1073
Copy link
Collaborator

You can run pre-commit now with meeseeks - "please pre-commit"

@blink1073 blink1073 added the bug label Mar 24, 2022
@codecov-commenter
Copy link

codecov-commenter commented Mar 24, 2022

Codecov Report

Merging #755 (b339cb1) into main (cd1b1b8) will decrease coverage by 0.11%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #755      +/-   ##
==========================================
- Coverage   70.68%   70.56%   -0.12%     
==========================================
  Files          62       62              
  Lines        7620     7627       +7     
  Branches     1218     1220       +2     
==========================================
- Hits         5386     5382       -4     
- Misses       1860     1877      +17     
+ Partials      374      368       -6     
Impacted Files Coverage Δ
jupyter_server/terminal/api_handlers.py 100.00% <100.00%> (+10.52%) ⬆️
jupyter_server/base/zmqhandlers.py 52.91% <0.00%> (-4.24%) ⬇️
jupyter_server/services/kernels/kernelmanager.py 80.63% <0.00%> (-1.91%) ⬇️
jupyter_server/services/kernels/handlers.py 58.86% <0.00%> (-0.22%) ⬇️

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 cd1b1b8...b339cb1. Read the comment docs.

@fcollonval
Copy link
Member Author

You can run pre-commit now with meeseeks - "please pre-commit"

Thanks @blink1073 - I was lazy and use github.dev instead of my local clone 😉


Windows tests are failing due to the project being checkout on a different drive than the one the server is running in.

Switching to draft for now.

it is removed in ServerApp from the root directory
@fcollonval fcollonval marked this pull request as draft March 24, 2022 13:40
@fcollonval fcollonval marked this pull request as ready for review March 24, 2022 14:47
@fcollonval
Copy link
Member Author

I don't know if the culling test are known to be flaky

@blink1073
Copy link
Collaborator

I don't know if the culling test are known to be flaky

Yes, cf #677

@fcollonval
Copy link
Member Author

Thanks @blink1073 then I guess this is ready.

@blink1073
Copy link
Collaborator

I'm still concerned about the Minimum Version test failure. I'll kick all of the jobs once the current GitHub outage incident is resolved.

@fcollonval
Copy link
Member Author

CI failure is due to jinja2 3.1 release that removed deprecated method contextfilter. This is breaking nbconvert.

@blink1073
Copy link
Collaborator

Kicking to pick up fixes from other PRs.

@blink1073 blink1073 closed this Mar 25, 2022
@blink1073 blink1073 reopened this Mar 25, 2022
@blink1073
Copy link
Collaborator

Failing test is unrelated, thanks!

@blink1073 blink1073 merged commit e0c126f into jupyter-server:main Mar 25, 2022
@fcollonval fcollonval deleted the ft/terminal-cwd branch March 25, 2022 17:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants