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

Improvements to "Standalone binaries" installation processes #122

Merged
merged 6 commits into from
Aug 30, 2018

Conversation

jonmmease
Copy link
Contributor

Overview

In preparation for the coming integration of orca into plotly.py, I reviewed and tested the "Standalone binaries" installation instructions for all three operating systems. This PR contains the updates from this review.

In all cases, the goal is to make sure the user ends up with an executable named orca on the permanent system PATH. This way the default plotly.py orca search logic will find it automatically.

I believe this is the last set of orca changes that we need before the plotly.py release. The Windows and Linux changes are documentation only. The Mac OS changes include both documentation and installer updates.

I think this is the last set of changes in orca that we need before the plotly.py release. I was picturing that we would release these changes as version 1.1.1 as soon as this PR is merged.

Windows

The Windows installation instructions have been updated to direct the user to add the orca executable to their PATH permanently. The prior instructions only added it to the PATH for the current Command Prompt session.

The actual environment variable instructions (for each Windows version) are delegated to a nice article on computerhope.com (https://www.computerhope.com/issues/ch001647.htm)

Linux

The Linux installation instructions have been updated to direct the user to create a symbolic link named orca that points to the AppImage. This way the user ends up with something name orca on their path (not orca-X.Y.Z-x86_64.AppImage).

Another computerhope.com reference was added to explain the concept of the PATH on Linux (https://www.computerhope.com/issues/ch001647.htm), along with a reference with some basic AppImage information (https://appimage.org/).

Mac OS

The logic to add orca to the system PATH has been refined as follows:

  • Don't make any changes if orca is already on the PATH.
  • Only copy orca.sh to /usr/local/bin if orca is installed as an application (not if it's installed through conda or npm). I think this will address the trouble folks are running into in Orca Error - sh: orca: command not found #120.
  • Perform the orca.sh copy with administrator privileges to avoid permission denied errors (See Permission denied downloading orca? #111). This is done using Apple Script, which launches a nice GUI password prompt.

The README instructions are also updated to reflect the new installation process.

…d orca

To help plotly.py discover orca, we need something on the PATH named `orca`,
not `orca-X.Y.Z...`.

Also add references about the Linux system PATH and AppImage
…anently

Prior instructions for adding orca to the PATH were not persistent between sessions.

The PATH manipulation instructions are delegated to a nice article on `computerhope.com`
Updated the Mac OS PATH installation logic to be a bit smarter:
 - Don't make any changes if `orca` is already on the `PATH`
 - Only copy `orca.sh` to `/usr/local/bin` if orca is installed as an application (not if it's installed through conda or npm)
 - Perform `orca.sh` copy with administrator privileges to avoid permission denied errors. This is done using Apple Script, which launches a nice GUI password prompt.

Updated README instructions to reflect the new installation process
try {
execSync(`cp ${source} ${target}`)
try {
execSync('which orca');
Copy link
Contributor

Choose a reason for hiding this comment

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

Ha interesting. I didn't think this would lead to an exception.

- Extract the `windows-release.zip` file.
- In the `release` folder, double-click on `orca Setup X.Y.Z`, this will create an orca icon on your Desktop.
- Right-click on the orca icon and select _Properties_ from the context menu.
- From the _Shortcut_ tab, copy the directory in the _Start in_ field.
Copy link
Contributor

Choose a reason for hiding this comment

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

To clarify, the Start in field is a path to a directory, correct? Not a path to a file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Correct, There's also a Target field that is the full path to the executable.

@etpinard
Copy link
Contributor

Thanks for doing this @jonmmease 🎉

Now docker-build-and-push is failing. Looks like svn can't resolve https://github.com/plotly/plotly.js.git/trunk/dist/extras/mathjax anymore. This probably has nothing to do with your PR.

This requires extra system libraries and xvfb.
@jonmmease
Copy link
Contributor Author

I just pushed some additional "Linux troubleshooting" instructions that cover the case of installing orca on Ubuntu Server (and they should at least be helpful to people working with other server distributions). This requires adding a handful of additional system libraries plus Xvfb.

In the instructions I suggest creating a shell script that runs the orca AppImage executable using xvfb-run. In my testing this works really well, and this way plotly.py won't need any Xvfb specific logic.

See what you think!

README.md Outdated
On Ubuntu Server, these additional libraries can be installed with:

```
sudo apt-get install libgtk2.0-0 libxtst6 libxss1 libgconf-2-4 libnss3 libasound2
Copy link
Contributor

@etpinard etpinard Aug 28, 2018

Choose a reason for hiding this comment

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

Could we instead tell users to install google-chrome (like in our image-server Dockerfile? That and libgtk2.0-0 / libgconf-2-4 should be enough to run orca smoothly

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I went this way to avoid the need to use anything outside of the standard repos. Are you thinking that the google-chrome approach would be more robust across distros?

Copy link
Contributor

Choose a reason for hiding this comment

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

I would prefer if the installation instructions match something that we can test.

Pointing users to the what we use in our image-server container should achieve that (as we test it on every push and on prod). But, perhaps our Dockerfile is a little too Ubuntu-centric? Do you think many people with try to user orca in non-Ubuntu server environments?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good. Done in a87754d

Let me know if you think of anything else!

@etpinard
Copy link
Contributor

In the instructions I suggest creating a shell script that runs the orca AppImage executable using xvfb-run. In my testing this works really well, and this way plotly.py won't need any Xvfb specific logic.

We use xvfb-run

xvfb-run --auto-servernum --server-args '-screen 0 640x480x24' ./bin/orca.js serve --request-limit=1000 --safe-mode $PLOTLYJS_ARG $@ 1>/proc/1/fd/1 2>/proc/1/fd/2 &

in our image-server container, so yeah it should work w/o any issues.

Instructions now match our deployment Dockerfile configuration
@etpinard
Copy link
Contributor

Thanks very much @jonmmease !

💃

@jonmmease
Copy link
Contributor Author

Sure thing! Hopefully this will help cut down on the number orca and plotly.py tickets 🙂

@etpinard
Copy link
Contributor

Hopefully this will help cut down on the number orca and plotly.py tickets

Yep, this PR is a solid step forward, until we take a stab at #92

@jonmmease
Copy link
Contributor Author

Now that we don't need to get #125 merged before plotly.py 3.2.0 is released, how does it sound to merge this PR and release it as 1.1.1? Would you have time to do the release by the end of the week? Thanks!

@etpinard
Copy link
Contributor

Would you have time to do the release by the end of the week

Sure. I'll do it before lunch today.

@etpinard etpinard merged commit a48dc5a into master Aug 30, 2018
@etpinard etpinard deleted the install_review branch August 30, 2018 15:31
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.

2 participants