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

Is pywin32 actually used? #281

Closed
mattip opened this issue Aug 23, 2020 · 5 comments · Fixed by #514
Closed

Is pywin32 actually used? #281

mattip opened this issue Aug 23, 2020 · 5 comments · Fixed by #514

Comments

@mattip
Copy link

mattip commented Aug 23, 2020

setup.py has a line to install pywin32 which is a huge package of c-extensions that does not work with PyPy. Is it actually required for jupyter_server?

@mattip
Copy link
Author

mattip commented Aug 23, 2020

Answering my own question: it seems it is used in secure_write, which calls win32_restrict_file_to_user https://github.com/jupyter/jupyter_server/blob/d1cb381195cacb82de530e57f0f8bebed07bf518/jupyter_server/utils.py#L226, which imports and uses win32api and win32security. This code is duplicated in jupyter_core. The jupyter_core implementation seems to be the code used according to a user @SleepyRoy who reported the issue in PyPy.

@kevin-bates
Copy link
Member

Correct - there was a bit of a migration exercise a while ago and this last step was not performed on this repo. jupyter_core should be used for this and setup.py should be updated to not include this dependency. There are a number of functions in utils.py that should get pulled from jupyter_core as well (e.g.. is_hidden()) and their usages updating accordingly - nice catch.

Would you be interested in making these changes?

@bollwyvl
Copy link
Contributor

bollwyvl commented Dec 10, 2020

Can concur that this would help downstream packaging, and that we should strive to keep this package's dependencies as cross-platform as possible for as long as possible. Perhaps:

  • on the 1.x line
    • provide shim imports in utils for everything that was moved from jupyter_core
    • add a deprecation warning
  • drop entirely on 2.x

cc @maartenbreddels

@stonebig
Copy link

stonebig commented May 9, 2021

version 2 seems far far away , could the jupyter-core simplification be anticipated a bit ?
... it still leaves the problem in one place, though :https://github.com/jupyter/jupyter_core/blob/master/jupyter_core/paths.py#L387.L389

@kevin-bates
Copy link
Member

I think that all that needs to happen is the dependency gets dropped from setup.cfg. The dependent methods were moved to jupyter_core via #382 but I failed to follow up on the removal of pywin32. I'll submit a PR for this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants