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

added support for installing plugins such as apoc at runtime #176

Merged
merged 1 commit into from
Jun 27, 2019

Conversation

eastlondoner
Copy link
Contributor

@eastlondoner eastlondoner commented Jun 20, 2019

This uses the same mechanism as neo4j desktop to fetch plugins from github.

It allows you to have apoc / graph algorithms / graph ql / etc. plugins by setting a json array:

e.g.

docker run --env NEO4JLABS_PLUGINS='["graph-algorithms", "apoc"]' neo4j

Plugins are always downloaded on container startup. A future improvement would be to use a hash (e.g. sha256 or md5) of the plugin to allow for caching of the dowloaded jar if the plugins dir is persisted.

Copy link
Member

@jexp jexp left a comment

Choose a reason for hiding this comment

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

Looks great!

@eastlondoner
Copy link
Contributor Author

I see what’s wrong with the tests - I need to rub docker with —user set (and add the apoc loading logic to all 3.x.

I’m taking the fam to the beach today, so prob won’t work on it till tomorrow at least

@kfreytag
Copy link

@eastlondoner Is there a reason why you're specifying a path for the plugins as opposed to an abstract name? As a user, I'd prefer to not have any sort of implementation details leak through to the command line here and just say NEO4JPLUGINS='["apoc","algos"]' or something like that. That also allows us to change the implementation under the sheets in the future without affecting the user surface.

@eastlondoner
Copy link
Contributor Author

eastlondoner commented Jun 26, 2019

@kfreytag it's not a path - it's the GitHub user / organisation where the plugin is coming from.
To avoid maintaining an extra dictionary of plugin name -> GitHub location and to hopefully be more future proof.

image

But you're totally right that this is an implementation detail - we could internally manage the mapping of plugin name -> where plugin exists and then it would provide a consistent interface to the user that doesn't depend on the implementation

@kfreytag
Copy link

By "path" I did mean GitHub path / repo, but yeah - the point remains. Let's abstract that away from the user. It also allows us to constrain which plugins can be "activated" in our official plugin as a NEO4JPLUGIN automatically.

@eastlondoner eastlondoner force-pushed the wget-apoc branch 5 times, most recently from 09f29cf to 7830225 Compare June 26, 2019 17:09
@eastlondoner
Copy link
Contributor Author

I changed the syntax to NEO4JLABS_PLUGINS='["apoc", "graph-algorithms"]'

I limited this change to versions >= 3.3

Copy link
Member

@jennyowen jennyowen left a comment

Choose a reason for hiding this comment

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

This looks fine.
It seems like the secure mode and this new setting might have some weird interaction. We probably need to unit test the combinations (that doesn't really affect this PR though).

@eastlondoner
Copy link
Contributor Author

@jennyowen it shouldn’t be too weird - mainly it adds /plugins to /data and /logs as places we need to be able to write to.

@eastlondoner eastlondoner merged commit 822872d into neo4j:master Jun 27, 2019
@jennyowen
Copy link
Member

@eastlondoner yeah, I realise the requirements aren't complicated, but following the logic through the docker-entrypoint.sh made for confusion.

@CarlColglazier
Copy link

CarlColglazier commented Oct 29, 2019

I am trying to get the graph algorithms working on my Docker deploy. It seems like this was reverted in ec4cbd2, but I can't figure out why. Does anyone know?

@jennyowen
Copy link
Member

@CarlColglazier it was reverted because it caused test failures and had issues with out CI infrastructure. We fixed it since then though, and the feature is available in 3.5.10 onwards.

@CarlColglazier
Copy link

@jennyowen Thank you!

I was having issues getting the plugins to download using docker-compose, but it seemed to work once I used my own Dockerfile:

FROM neo4j:3.5.11

ENV NEO4JLABS_PLUGINS '["apoc", "graph-algorithms"]'

CMD ["neo4j"]

@eastlondoner
Copy link
Contributor Author

@CarlColglazier I don't think that will work. Setting the CMD as neo4j executes the neo4j binary directly, bypassing the extra logic that we have in the docker entry point.

Can you share the docker-compose file that you were having problems with?

@joelduerksen
Copy link

joelduerksen commented Jan 20, 2020

@eastlondoner this is a really nice feature, and I immediately switched over to using it. Very clean, I was able to clean the wgets out of my scripts.

Now I'm working to move to neo4j 4.0, graph-algorithms plugin not loading, and notice that it assumes referenced libraries will always have the same version as the neo4j db. I took a look at the code in docker-entrypoint.sh...

It might be nice to be able to specify a version (if there is a compatible jar). In the past some jars were compatible (across minor update versions)..

** graph-algo 3.5.14.0 does not appear to work with neo4j 4.0. Adding the jar causes neo4j to shutdown with a seemly unrelated error (but if I remove the jar, it starts OK). It was a big update, so this is not a surprise.

@eastlondoner
Copy link
Contributor Author

@joelduerksen the docker container looks up the correct plugin version to match the neo4j version from the plugin's repo. In this case: https://github.com/neo4j-contrib/neo4j-graph-algorithms/raw/master/versions.json

There is currently no published graph-algos version that is compatible with Neo4j 4.0
@AliciaFrame can provide more information about graph-algorithms compatibility...

@AliciaFrame
Copy link

@joelduerksen - the graph algorithms library (3.5.14) isn't compatible with Neo4j 4.0, and won't be.

We'll be releasing an updated Graph Data Science, or GDS, library in the next month or so (new surface, officially supported, improved algorithms and infrastructure) and formally deprecating the older algos library. We're currently planning to provide a Neo4j 4.0 compatible release by April of this year.

Sorry for the confusion!

@davidburkitt
Copy link

This feature doesn't seem to be valid for docker-compose. Can you provide a working example if this should be the case please?

@eastlondoner
Copy link
Contributor Author

@davidburkitt could you possibly provide the docker compose file that you are using which isn't working ? I guess you already have one. It will take me a little while to make one from scratch.

@davidburkitt
Copy link

davidburkitt commented Aug 11, 2020

version: '3.7'
services:
web:
container_name: neo4j_web
image: neo4j:latest
restart: unless-stopped
ports:
- "7474:7474"
- "7687:7687"
volumes:
- ${HOME}/neo4j/data:/data
- ${HOME}/neo4j/logs:/logs
- ${HOME}/neo4j/import:/import
environment:
- NEO4JLABS_PLUGINS='["apoc"]'
- NEO4J_AUTH=${CREDS}
- NEO4J_dbms_security_procedures_unrestricted='apoc.\\\*'
- NEO4J_apoc_export_file_enabled=true
- NEO4J_apoc_import_file_enabled=true
- NEO4J_apoc_import_file_use__neo4j__config=true

@eastlondoner
Copy link
Contributor Author

eastlondoner commented Aug 11, 2020

@davidburkitt The problem here is to do with string quoting in yaml. You do not need to quote the values of the env vars being set in yaml.
So this is fine:

- NEO4JLABS_PLUGINS=["apoc"]

If you do want to quote them then you have to quote the entire string:

- 'NEO4JLABS_PLUGINS=["apoc"]'

n.b. you should not need to add NEO4J_dbms_security_procedures_unrestricted if you are using NEO4JLABS_PLUGINS

So this should work fine:

version: '3.7'
services:
  web:
    container_name: neo4j_web
    image: neo4j:latest
    restart: unless-stopped
    ports:
      - "7474:7474"
      - "7687:7687"
    volumes:
      - ${HOME}/neo4j/data:/data
      - ${HOME}/neo4j/logs:/logs
      - ${HOME}/neo4j/import:/import
    environment:
      - NEO4JLABS_PLUGINS=["apoc"]
      - NEO4J_AUTH=${CREDS}
      - NEO4J_apoc_export_file_enabled=true
      - NEO4J_apoc_import_file_enabled=true
      - NEO4J_apoc_import_file_use__neo4j__config=true

@davidburkitt
Copy link

I think this might be an issue with string quotes in Docker on macOS as I'm testing applications locally on a MacBook Pro. I've tried every conceivable combination of quotes/escapes/no quotes without success.

@eastlondoner
Copy link
Contributor Author

@davidburkitt here's a screenshot from my Mac. I am using Docker Desktop for Mac. You can see it log the message installing plugin 'apoc' before starting neo4j

Are you seeing any particular errors?

image

@davidburkitt
Copy link

@eastlondoner apologies - I should have stated; I'm trying to run locally but detached 'docker-compose up -d'. It's entirely possible I'm missing the point and conceptually that's incorrect?

@eastlondoner
Copy link
Contributor Author

@davidburkitt also fine:
image

Could you provide some more information on what error message you're getting or the way that it's not working?

@AliciaFrame
Copy link

AliciaFrame commented Aug 11, 2020 via email

@davidburkitt
Copy link

@eastlondoner Your example works perfectly - ignore my previous - must have been a local versioning issue maybe. Removing quoting/escapes from the env vars was the fix. Thanks

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.

8 participants