Miscellaneous Improvements: yq v4; TinkerIoRegistry; cross-platform build

[phreed]

I forked the project and am working on several improvements.
https://github.com/babeloff/janusgraph-docker

I would like as many of these to make it into the main repo as possible.
Could I get some guidance on the best way to proceed?)

• upgrade yq to v4.x
• add TinkerIoRegistry to the gremlin-server.yaml
• switch from bash scripts to Gradle
• remote-objects.yaml

YQ

The new version is much more capable than the prior.
This change leads to an overhaul of docker-entrypoint.sh and expansion of the configuration of docker environment variables e.g. gremlinserver.* -> gremlinserver%1.* (or something like that)

TinkerIoRegistry

The ability to return subgraphs is not possible because the TinkerIoRegistryV3d0 is not included in the gremlin-server.yaml.
This could be overcome with the yq update mentioned above. In the short run the following could be added.

serializers:
...
  - className: org.apache.tinkerpop.gremlin.driver.ser.GryoMessageSerializerV3d0
    config:
      ioRegistries:
        - org.janusgraph.graphdb.tinkerpop.JanusGraphIoRegistry
        - org.apache.tinkerpop.gremlin.tinkergraph.structure.TinkerIoRegistryV3d0
...

Gradle

bash is flaky on Windows.
Gradle works across platforms and has Docker plugins.

remote-objects.yaml

It has the wrong host (localhost rather than janusgraph)

[mkaisercross]

Hi @phreed.

The config file change you mentioned can be achieved with the following:

docker run -it -e gremlinserver.serializers[2].config.ioRegistries[+]=org.apache.tinkerpop.gremlin.tinkergraph.structure.TinkerIoRegistryV3d0 janusgraph/janusgraph:latest janusgraph show-config

I took a quick look at yq v4 and I agree the new syntax looks more capable. As I mentioned above v3 can solve your problem so I am not 100% sure it is necessary to update but here are my thoughts on what might be a way to do the update while keep the work minimal.

It looks like the new syntax would be something like yq eval ".${envkey} = '${envval}'" gremlin-server.yaml. But supporting all the piping and select statements would be difficult to translate to our current method since there is no clear key and value that can correspond to an environment variable and it's value. So my suggestion would be to keep the existing syntax we currently have (docker run -e gremlinserver.foo=bar) and in addition we can add something like docker run -e gremlinserver%e="..." where the '%e' part would let the script know to evaluate the string. Basically just pass it along to the yq i.e. yq eval ${envval} gremlin-server.yaml. The only problem with this however is it would only let the user evaluate one string so maybe we could add some separator between expressions within the string. These are just my initial thoughts and would be interested what others think about this.

[mkaisercross]

Also I missed your suggestion to have the syntax be something like gremlinserver%1 and I am assuming you could have gremlinserver%2, gremlinserver%3, etc. I think that could work as well but the user would have to keep track of the number when adding new envars which could be a bit difficult for the user to keep track of if the envars are coming from different files or given in command line. Regardless of the way this eval method could be implemented I would prefer that we kept the old syntax if possible (e.g. gremlinserver.graphs.graph=...) as another way to edit the config in addition to the more advanced eval method. IMO feels a little more friendly for people just doing basic configuration.

[phreed]

@mkaisercross
I have been working on an alternate janusgraph-docker based primarily on yq v4.
You can see it here. https://github.com/babeloff/janusgraph-docker2

There are a few other things that I wanted:

• adding sample compose configurations
• producing documentation via asciidoc
• consistent editing of configuration files via yq
• building is done with gradle

The editing of the janusgraph-foo.properties files had a different syntax than did the gremlin-server.yaml
This is due to one being a *.properties and the other being a *.yaml
I asked mikefarah (yq author) about producing *.properties from *.yaml and he added that capability.
During the docker image construction the *.properties are converted to *.yaml files with props2yaml.
Then, the docker-entrypoint.sh script update the *.yaml using yq which outputs the result as *.properties

[FlorianHockmann]

Reading this issue again, I think that two of the issues you mentioned don't apply any more or should be handled outside of the Docker image:

• remote-objects.yaml is now supported, thanks mostly to a PR from yourself: Also support remote-objects.yaml #96.
• TinkerIoRegistry: I think this is something we should support in general and not only for Docker, but we also already have an issue for that in the main repo: Serialization of TinkerGraph for subgraph() step should be supported by default janusgraph#2843.

Updating to yq v4 is something we should definitely do. I've created a dedicated issue for that: #101. If you want to contribute your changes here, then a PR would of course we welcome!

This leaves the suggested move from Bash to Gradle: Could you please create a dedicated issue for that where you describe which scripts you want to migrate (only the build scripts or also the scripts that are used by the container itself, e.g., docker-entrypoint.sh?) and what advantages you see?

I'm closing this issue here now as it hasn't seen any activity lately and because I think that it's easier to follow the progress if we have individual issues for these different aspects. Hope this doesn't discourage you, @phreed, and again: Feel free to create an issue for the move from Bash to Gradle and to contribute PRs with your suggested improvements!

[phreed]

Just the build scripts would be migrated to gradle.
I will open a separate issue #102 as you suggest to cover converting from bash building to gradle.