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

Release official Docker image + pixel exact image tests #222

Merged
merged 16 commits into from Jun 21, 2019

Conversation

@antoinerg
Copy link
Contributor

commented Mar 27, 2019

Closes #187

@scjody suggested we reuse our existing Dockerfile to publish our official Docker images. In order to do so, I had to adapt its ENTRYPOINT to support CLI invocation (see #187 (comment) for details). If the first argument is either --version, --help or graph we assume Orca is used as a CLI. If the first argument is serve or anything else, we launch the server as we used to do.

  • Publish Docker image as plotly/orca
  • Support CLI invocation
  • Advertise our Docker image in the documentation
  • Add image tests

image tests

The inclusion of image tests in this PR eliminates the risk of missing dependencies in the final Docker image. By using software rendering, we get reproducible images that we can compare pixel per pixel with a set of baselines in an exact manner. By visually testing all image formats (pdf, eps, svg, png, emf), we catch missing system dependencies such as Poppler, Inkscape, Mathjax or missing fonts. Hopefully, this will catch regressions whenever we perform system upgrades.

  • document image tests in CONTRIBUTING.md
  • runs automatically on CircleCI as a final job

@plotly/devops please have a look at the new entrypoint.

@jonmmease

This comment has been minimized.

Copy link
Collaborator

commented Mar 28, 2019

Thanks for working on this @antoinerg, this is going to be really helpful!

I'd like to see if we can work through how Python/R/Julia users could take advantage of orca as a docker image without each library needing to include docker related logic. Would it be possible to distribute a shell script, orca.sh, that presents that same interface as the native orca binary? Something like

if orca graph then call docker run -i quay.io/plotly/orca graph "$@"

if orca serve --port=8123 then call docker run -d -p 8123:8123 quay.io/plotly/orca "$@"

The R and Julia libraries use the orca graph command line API, while plotly.py uses the orca serve API with --port and --graph-only options. If we had a shell script like this for users to put on their path then I think all of the client libraries would work without modification. Can docker pull add something to the PATH or would that have to be a manual step?

What do you think?

@antoinerg

This comment has been minimized.

Copy link
Contributor Author

commented Mar 28, 2019

Thanks for working on this @antoinerg, this is going to be really helpful!

I'm glad to see lots of people are excited about this one!

I'd like to see if we can work through how Python/R/Julia users could take advantage of orca as a docker image without each library needing to include docker related logic. Would it be possible to distribute a shell script, orca.sh, that presents that same interface as the native orca binary?

That is certainly one of the goal here. Making an orca.sh shell script calling Docker as you suggested would be great and I will try to add one to this PR.

Can docker pull add something to the PATH or would that have to be a manual step?

As far as I know, it cannot. It would have to be a manual step.

@scjody

This comment has been minimized.

Copy link
Contributor

commented Mar 28, 2019

Sounds like a great idea to me!

Minor request: if you're adding a tool that users are expected to run, it should not have a .sh (or .bash, .py, etc.) extension. Rationale

Also note that we have a company-wide bash style guide (mostly incorporates Progrium's).

antoinerg added some commits Apr 16, 2019

@antoinerg antoinerg force-pushed the docker-release branch from 9a68805 to ac00cfd Apr 16, 2019

.gitignore Outdated Show resolved Hide resolved

@etpinard etpinard added this to the v1.3 milestone Apr 17, 2019

add shell script to call Orca in Docker
It forwards a call to Orca in Docker in a seamless fashion by:
- binding the container to the host's networking stack
- bind-mounting the current folder into the container at the same path
- setting the current directory to the current path
@antoinerg

This comment has been minimized.

Copy link
Contributor Author

commented Apr 26, 2019

@jonmmease I wrote a little shell script in commit 94bb0a4 to call Orca in Docker. Please see the commit message for details. Also, can you test it out on MacOS and adapt it if it doesn't work. Anyway, let me know what you think!

@antoinerg antoinerg changed the title Release official Docker image Release official Docker image + pixel exact image tests Apr 26, 2019

.gitignore Outdated Show resolved Hide resolved
test/image/render_mocks_cli Outdated Show resolved Hide resolved
@antoinerg

This comment has been minimized.

Copy link
Contributor Author

commented May 23, 2019

Any comments for this PR? @etpinard @jonmmease @plotly/devops

Thank you all!

@etpinard

This comment has been minimized.

Copy link
Member

commented May 23, 2019

💃 from me

@antoinerg antoinerg force-pushed the docker-release branch from e29de4b to c2ec25f Jun 21, 2019

@antoinerg antoinerg merged commit f409622 into master Jun 21, 2019

9 checks passed

ci/circleci: docker-build-and-push Your tests passed on CircleCI!
Details
ci/circleci: electron-pack-and-release Your tests passed on CircleCI!
Details
ci/circleci: test-image Your tests passed on CircleCI!
Details
ci/circleci: test-node-v6 Your tests passed on CircleCI!
Details
ci/circleci: test-node-v8 Your tests passed on CircleCI!
Details
continuous-integration/appveyor/branch AppVeyor build succeeded
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@antoinerg antoinerg deleted the docker-release branch Jun 21, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.