Page MenuHomeSoftware Heritage

docker setup: simplify and uniform to SWH path conventions
ClosedPublic

Authored by zack on Wed, Jul 24, 6:49 PM.

Details

Reviewers
seirl
haltode
Summary

Major simplification pass on the docker documentation, Dockerfile, and scripts.

Main aim was to simplify the steps needed (as documented in docker.rst) and
reduce the amount of redundant information that need to be provided by the
user.

As this required a major overhaul, this revamp also includes:

  • porting path conventions to SWH standards (e.g., everything is now under /srv/softwareheritage/graph, even in the container)
  • sanitizing the main shell script

Diff Detail

Repository
rDGRPH Graph service
Branch
feature/docker-simplification
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 7071
Build 9966: arc lint + arc unit

Event Timeline

zack created this revision.Wed, Jul 24, 6:49 PM
zack edited reviewers, added: seirl, haltode; removed: Reviewers.Wed, Jul 24, 6:49 PM
zack updated this revision to Diff 5988.Thu, Jul 25, 5:02 PM
  • docker doc: further shorten CLI examples using relative paths
zack updated this revision to Diff 6010.Sun, Jul 28, 5:37 PM
  • docker doc: drop the list of files generated by Setup class
zack updated this revision to Diff 6011.Sun, Jul 28, 7:05 PM
  • docker doc: add --publish to the run invocation

Awesome, thanks a lot for this diff!

Note that you also need to update the java/server/src/test/dataset/generate_graph.sh script since the Docker environment has changed.

zack planned changes to this revision.Mon, Jul 29, 4:58 PM

Tnx, will do.

(Don't hesitate to explicitly request changes in cases like this in the future :))

zack updated this revision to Diff 6032.Tue, Jul 30, 3:32 PM
  • tests: update generate_graph.sh to match new docker layout
zack added a comment.Tue, Jul 30, 3:37 PM

Note that you also need to update the java/server/src/test/dataset/generate_graph.sh script since the Docker environment has changed.

This is now done.

Note that this diff still makes the test fail after re-running generate_graph.sh, because apparently the mappings are no longer generated. (Although it doesn't seem compress_graph.sh has ever generated them, so I'm not sure it's related to this diff [?]).
Anyway, if that's OK with you, please accept this diff as is (unless other issues are affected by it), and fix generate_graph.sh later on in a subsequent commit, after it lands.

haltode accepted this revision.Tue, Jul 30, 4:12 PM
In D1768#41626, @zack wrote:

Note that this diff still makes the test fail after re-running generate_graph.sh, because apparently the mappings are no longer generated. (Although it doesn't seem compress_graph.sh has ever generated them, so I'm not sure it's related to this diff [?]).

The mapping has always been a separate command but we can indeed add it to the generate_graph.sh script.

This revision is now accepted and ready to land.Tue, Jul 30, 4:12 PM
zack added a comment.Tue, Jul 30, 5:57 PM

The mapping has always been a separate command but we can indeed add it to the generate_graph.sh script.

Oh, ok, that explains it.

That means that the README on how to recreate the testdata isn't correct at present (or at least it's incomplete).

Integrate this into generate_graph.sh is a good idea. When it's done, we can also further simplify docs/docker.rst.

In D1768#41639, @zack wrote:

The mapping has always been a separate command but we can indeed add it to the generate_graph.sh script.

Oh, ok, that explains it.
That means that the README on how to recreate the testdata isn't correct at present (or at least it's incomplete).
Integrate this into generate_graph.sh is a good idea. When it's done, we can also further simplify docs/docker.rst.

Ok now I remember why this was separated, I was waiting on shipping the swh-graph.jar so I could add the step inside the compress_graph. I am tracking this issue in T1941.