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

don't make numpy a hard requirement #163

Closed
wants to merge 1 commit into from
Closed

don't make numpy a hard requirement #163

wants to merge 1 commit into from

Conversation

sdague
Copy link

@sdague sdague commented Mar 27, 2015

the websockify code works without numpy. It will be slower, however it
only seems to be used if you are directly processing masked buffers in
python. However, the current code hard enforces numpy in setup.py, so
if you install the package you forcably get numpy.

In OpenStack we use the websockify proxy a lot of places where the
numpy paths aren't touched. Forcing numpy to be installed in all these
circumstances is somewhat overkill.

This would take it out of forced requirements.

the websockify code works without numpy. It will be slower, however it
only seems to be used if you are directly processing masked buffers in
python. However, the current code hard enforces numpy in setup.py, so
if you install the package you forcably get numpy.

In OpenStack we use the websockify proxy a lot of places where the
numpy paths aren't touched. Forcing numpy to be installed in all these
circumstances is somewhat overkill.

This would take it out of forced requirements.
@DirectXMan12
Copy link
Member

I am unclear as to why the numpy paths would be untouched. RFC 6455 states that "a client MUST mask all frames that it sends to the server". According to https://developer.mozilla.org/en-US/docs/WebSockets , RFC6455 support has been in place in most browsers for a while.

Could you please clarify where you use websockify that the numpy paths would not be touched?

@sdague
Copy link
Author

sdague commented Mar 27, 2015

Ah, you are right, I didn't understand all the implications here. Thanks.

@sdague sdague closed this Mar 27, 2015
@sdague
Copy link
Author

sdague commented Mar 27, 2015

I do wonder if that's the case why is numpy optional in the code?

@DirectXMan12
Copy link
Member

I suspect that it's because websockify was designed to be compatible with a wide range of environments and Python versions, so of which may not have had numpy support at all. If someone wanted to run websockify in one of these environments, hypothetically they could clone the repository and run websockify from the repository, without having to actually patch any code (they just couldn't use setup.py or pip install).

openstack-gerrit pushed a commit to openstack/openstack that referenced this pull request Apr 1, 2019
* Update devstack from branch 'master'
  - Merge "Don't install numpy packages"
  - Don't install numpy packages
    
    numpy is a python requirement of the websockify package (it appears
    there was some disucssion over *removing* this in [1], but did not
    happen).  Possibly these packages were installed a long time ago
    before wheel support as it was taking a long time to build.  But we
    have wheels today, and later versions than the distro provides are
    being dragged in anyway.  Remove all distro installs.
    
    [1] novnc/websockify#163
    
    Change-Id: I322dd9e1a07d8ce03c26cf3fcccebd6e21282fe4
openstack-gerrit pushed a commit to openstack/devstack that referenced this pull request Apr 1, 2019
numpy is a python requirement of the websockify package (it appears
there was some disucssion over *removing* this in [1], but did not
happen).  Possibly these packages were installed a long time ago
before wheel support as it was taking a long time to build.  But we
have wheels today, and later versions than the distro provides are
being dragged in anyway.  Remove all distro installs.

[1] novnc/websockify#163

Change-Id: I322dd9e1a07d8ce03c26cf3fcccebd6e21282fe4
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.

None yet

2 participants