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

Dockerfile for playground #178

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

igor-starostenko
Copy link

  • Created Dockerfile for playground
  • Fixed /jslt POST request
  • Added docs

@larsga
Copy link
Collaborator

larsga commented Feb 15, 2021

This is an interesting idea, but I wonder a little about the value of it. I can see the value of using Docker for complicated tools consisting of several different processes and so on. But here we're just spinning up a jar file and connecting to a port.

I do think there is value in putting all the pieces together as concisely as this, though, because it really lowers the bar for anyone who wants to run the playground locally. Even if they must have docker, which not everyone does.

But, I wonder, could we not simply do this with a shell script?

@igor-starostenko
Copy link
Author

igor-starostenko commented Feb 15, 2021

@larsga There are several reasons dockerizing the playground server can be beneficial.

  1. For someone who is not very familiar with Java and doesn't have it properly set up but wants to run the playground locally to test the latest JSLT code. A shell script may or may not work on certain operating systems or java versions. Docker helps isolate and simplify the configuration to just one OS and environment setup.
  2. I think JSLT has much more potential to be used as a HTTP service that would transform JSON on demand. And playground already has that functionality built in. Besides the html page it also accepts the POST request with jslt and json properties. I'm currently using it in my company this way: deployed the container in a cloud and make it available for other internal services to transform data.

Copy link
Contributor

@biochimia biochimia left a comment

Choose a reason for hiding this comment

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

I think this is a good addition to the project. Personally, I already run JDK/JRE inside docker in my laptop, but it's nice if these images can be published automatically on releases to make it easier to play with the language.

Given that it's now possible to publish docker containers directly to GitHub, integrating this with GitHub Actions to do so could also be a nice addition.


COPY . .

RUN ./gradlew :playground:shadowJar
Copy link
Contributor

Choose a reason for hiding this comment

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

I suggest switching to a multi-stage build, so that the final docker container has only the JSLT jar on top of a slimmer JRE image. As it stands the final container will contain the full JDK plus any intermediate build artifacts.

Another alternative would be to build the jar outside docker and then package it in a vanilla JRE image directly. This has the downside that the Dockerfile and a running docker setup are no longer self-sufficient for bootstrapping.

Copy link
Collaborator

Choose a reason for hiding this comment

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

This sounds very reasonable to me.

The number of dependencies is very low. Perhaps we could just get them from Maven Central with wget or something?

+--- project :core
|    \--- com.fasterxml.jackson.core:jackson-databind:2.12.0
|         +--- com.fasterxml.jackson.core:jackson-annotations:2.12.0
|         \--- com.fasterxml.jackson.core:jackson-core:2.12.0
\--- org.eclipse.jetty:jetty-server:9.4.29.v20200521
     +--- javax.servlet:javax.servlet-api:3.1.0
     +--- org.eclipse.jetty:jetty-http:9.4.29.v20200521
     |    +--- org.eclipse.jetty:jetty-util:9.4.29.v20200521
     |    \--- org.eclipse.jetty:jetty-io:9.4.29.v20200521
     |         \--- org.eclipse.jetty:jetty-util:9.4.29.v20200521
     \--- org.eclipse.jetty:jetty-io:9.4.29.v20200521 (*)

We could also host the shadowJar as a github release or something, but this seems maybe simpler and more straightforward?

@larsga larsga added enhancement New feature or request good first issue Good for newcomers labels May 26, 2021
@larsga
Copy link
Collaborator

larsga commented May 26, 2021

I think @igor-starostenko's rationale makes sense, and if more people are interested then I think we should just do it. I'm not using Docker, so I'm not the person to do it. Could you take it on, @biochimia?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants