Details

Reviewers

Commits

rDENV8871e9cd32eb: swh-scheduler: add SWH_CONFIG_FILENAME and SWH_SCHEDULER_CONFIG_FILE env vars
rDENVc56678130229: readme: integrate the docker-based development setup from the main's doc
rDENVd192c1cde3d7: docker-compose: move the swh-indexer-journal-client in the journal "section"
rCDFD8871e9cd32eb: swh-scheduler: add SWH_CONFIG_FILENAME and SWH_SCHEDULER_CONFIG_FILE env vars
rCDFDc56678130229: readme: integrate the docker-based development setup from the main's doc
rCDFDd192c1cde3d7: docker-compose: move the swh-indexer-journal-client in the journal "section"

Summary

Also:

docker-compose: move the swh-indexer-journal-client in the journal "section"

swh-scheduler: add SWH_CONFIG_FILENAME and SWH_SCHEDULER_CONFIG_FILE env vars

in all the swh-scheduler related services. Also configure the CELERY_BROKER_URL env var.

These allow to use the swh-scheduler directly in the container without
any cmdline option, as well as the celery command.

Diff Detail

Repository

rCDFD Dockerfiles for developers

Lint

Automatic diff as part of commit; lint not applicable.

Unit

Automatic diff as part of commit; unit tests not applicable.

Event Timeline

douardda created this revision.Mar 5 2019, 3:33 PM

Harbormaster completed remote builds in B4543: Diff 3834.Mar 5 2019, 3:33 PM

Thanks for this doc refactoring too!

I've noted down a bunch of suggestions, but they are mostly minor issues and/or suggestions for compacting the provided information.

README.md
14 ↗	(On Diff #3834)	Minor rewording/typo fixes: Running a Software Heritage instance on your machine can consume quite a bit of resources: if you play a bit too hard (e.g., if you try to list all GitHub repositories with the corresponding lister), you may fill your hard drive, and consume a lot of CPU, memory and network bandwidth.
27 ↗	(On Diff #3834)	s/the//
33 ↗	(On Diff #3834)	I suggest to stick to a single way of having the repo, rather than mentioning multiple possibilities. It will shorten the text, and savvy users will find out how to do the non-standard way by themselves. As it's also needed below, we should probably just recommend cloning swh-environment, and pointing to the relevant section of the developer-setup guide instead of duplicating the info here.
36 ↗	(On Diff #3834)	I think this link will be dangling. The anchor in developer-setup.rst is .. _developer-setup:
45 ↗	(On Diff #3834)	s/but using code/with overrides/
85 ↗	(On Diff #3834)	s/try to execute the/run/ also, typo in "docker-co;pose"
270 ↗	(On Diff #3834)	This and the following exec actions are not shown as being run from the swh-docker-dev, so they probably fail unless the user configure docker-compose to look for the right file. We should either change them to be shown as run from ~/swh-environment/swh-docker-dev/ as before, or document how to point to the right compose file and use it throughout the guide.
387 ↗	(On Diff #3834)	The title needs rewording. (I'm not entirely sure what you want to convey here, so I'm not proposing an alternative formulation yet.)
389–412 ↗	(On Diff #3834)	This is duplicate information from the developer-setup guide. I suggest to just link to the relevant section of it, and tell readers to come back here once they've obtained swh-environment. (Also: see the suggestion to just anticipate this at the beginning of this guide, as the recommended way to obtain swh-docker-dev.)
449–451 ↗	(On Diff #3834)	Ugh. If there's a way to make pip ignore those files, it'd be much nicer, but meh, it's a separate issue :-)
454–462 ↗	(On Diff #3834)	s/swh/SWH/ (I generally prefer to spell it fully as Software Heritage, but if we really go for acronyms let's make it clear they are so) s/virtual/virtualenv/ in the last line (I guess)

This revision now requires changes to proceed.Mar 6 2019, 11:06 AM

douardda marked 9 inline comments as done.Mar 8 2019, 10:18 AM

douardda added inline comments.

README.md
33 ↗	(On Diff #3834)	This is the README file of the swh-docker-dev repo, so I wanted to be fairly self-sustained, especially the quick-start guide. So I'm not sure to follow you on here. It's not really a quick start guide if it depends on the developer setup guide to have been applied.
36 ↗	(On Diff #3834)	I use here a reference which is declared at the end of this README file (because I use it several times)... which is markdown, not rest. As is, this README file is not included in the global documentation, which I'm trying to do in another (unsubmitted) diff. We could decide to put most of the content of this README file in the docs/ directory, convert it to rest, and handle it as any other swh-xxx package...
270 ↗	(On Diff #3834)	in a differente version of the doc, this was handled by the 'make your life easier' section in which the COMPOSE_FILE env var is declared. But it's now below so yes, it won't work as is.

douardda retitled this revision from docker-compose: move the swh-indexer-journal-client in the journal "section" to readme: integrate the docker-based development setup from the main's doc.Mar 8 2019, 10:22 AM

douardda edited the summary of this revision. (Show Details)

rebased and fix some of zack's comments

Harbormaster completed remote builds in B4562: Diff 3850.Mar 8 2019, 10:28 AM

zack added inline comments.Mar 8 2019, 11:20 AM

README.md
33 ↗	(On Diff #3834)	Fair enough about the external dependency. Still, I don't see the point of recommending two alternatives way of getting the docker-compose repo, especially if later on in the guide we tell the reader that he needs the complete one (i.e., having swh-environment). We should just tell user from the start to clone the entire swh-environment and be done with it. Less choices to be made for the reader, and less text in the guide.
53 ↗	(On Diff #3850)	missing trailing 't' in the URL
87 ↗	(On Diff #3850)	I'm guessing this might become obsolete after stuff like D1227 and D1228, but we can always fix this later.

douardda marked 3 inline comments as done.Mar 8 2019, 12:03 PM

douardda added inline comments.

README.md
33 ↗	(On Diff #3834)	well, this later part is kind of an advanced usage of the docker stuff, and is not necessary. It's only useful for someone who wants to be able to run docker services with overloaded sources. Running the SWH platform in docker containers does not require to clone a dozen git repos. This is only needed for the developer. This README guide does not necessary target developers only. So my intention was to keep the quick start as simple as possible, but keeping the doors open for the user interested in hacking the code later on.
87 ↗	(On Diff #3850)	Hopefully it is now. Let's keep it for now until we are confident enough there is no other caveats in there.