Page MenuHomeSoftware Heritage

docker-compose: Add the service dependencies
ClosedPublic

Authored by ardumont on Dec 12 2018, 5:53 PM.

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

ardumont created this revision.Dec 12 2018, 5:53 PM
vlorentz accepted this revision.Dec 12 2018, 6:11 PM
vlorentz added a subscriber: vlorentz.
vlorentz added inline comments.
Makefile
9 ↗(On Diff #2563)

Just repeat docker-compose up, it's better than calling make from itself

README.md
27 ↗(On Diff #2563)

+ "that stores the Merkle DAG"

This revision is now accepted and ready to land.Dec 12 2018, 6:11 PM
vlorentz added inline comments.Dec 12 2018, 6:12 PM
Makefile
9 ↗(On Diff #2563)

Also, I wouldn't expect a target named rebuild to actually run the stuff. Maybe rename it to clean-run`?

ardumont marked an inline comment as done.Dec 12 2018, 6:32 PM
ardumont added inline comments.
Makefile
9 ↗(On Diff #2563)

Just repeat docker-compose up, it's better than calling make from itself

Right.

Also, I wouldn't expect a target named rebuild to actually run the stuff. Maybe rename it to clean-run`?

Yes.
I'd be even for removing that line.

clean:
        docker-compose down --volumes

People who wants to do that can then do:

make clean build run

that's cleaner and expected, what do you think?

ardumont marked an inline comment as done.Dec 12 2018, 6:34 PM
ardumont added inline comments.
Makefile
9 ↗(On Diff #2563)

or up and down (vagrant users would not be disturbed, vagrant up and vagrant down are used there ;)

ardumont updated this revision to Diff 2564.Dec 12 2018, 6:36 PM
  • README: Finish the sentence
  • Makefile: Untangle the commands
vlorentz added inline comments.Dec 12 2018, 7:22 PM
Makefile
4 ↗(On Diff #2564)

Why did you rename it? It's common for the first make target to be called all. (And having a target named build is kind of redundant, as Make's job is to build stuff)

9 ↗(On Diff #2563)

run must depend on build.

ardumont marked 2 inline comments as done.Dec 12 2018, 7:57 PM
ardumont added inline comments.
Makefile
4 ↗(On Diff #2564)

I fight on speed and somehow mixed stuff.
Changing that.

9 ↗(On Diff #2563)

I don't see why we should need to have that here.

make run will systematically trigger a build even though nothing changed.

I'd be more for having to trigger make build run the first time (and when we need to change something).
And make run when we only want to run stuff.

If you have noise in the swh-environment (and some people have), that slows all the damn things.
And we won't see it immediately (like i did, took me a day).

I added the .dockerignore in swh-environment:

*/*.git
**/*.pyc
*/.tox
*/.hypothesis
swh-web/node_modules
!swh-web/.git
.venv/
packages/

(not in that repository as that does not work either).

and it helped a little but that was not enough (i still had noise to cleanup...).

ardumont updated this revision to Diff 2568.Dec 13 2018, 10:02 AM
  • Makefile: Revert to origin targets but explicit them
ardumont marked an inline comment as done.Dec 13 2018, 10:03 AM
ardumont added inline comments.
Makefile
9 ↗(On Diff #2563)

for the .dockerignore, i've opened a D807 for it btw (for completeness' sake)

This revision was automatically updated to reflect the committed changes.
vlorentz added inline comments.Dec 13 2018, 11:32 AM
Makefile
9 ↗(On Diff #2563)

I don't see why we should need to have that here.

Because building is a dependency of running.

make run will systematically trigger a build even though nothing changed.

Yes, but Docker caches intermediate images, so it won't actually have to rebuild it.

I'd be more for having to trigger make build run the first time

There is no guarantee that build will run before run

Because building is a dependency of running.

Yes, i agree but in that case...

Yes, but Docker caches intermediate images, so it won't actually have to rebuild it.

Yes, but in regards to stuff fetching from the internet moving targets (pypi, npm), that's not caching anything.
Debian seems fine as that moves less (in stable ;)

I'm in the middle of building the swh-web one and that's not caching those steps.
Every single time, it does it all again.
That's slow.

Also, note that for now, in the current master, there are only 3 containers.
We will have much more.

There is no guarantee that build will run before run

I'm curious, how so?

There is no guarantee that build will run before run

I'm curious, how so?

 val@particle  /tmp  cat Makefile
%:
	echo $@
 val@particle  /tmp  make a b c d e f -j
echo a
echo b
echo c
echo d
echo e
a
echo f
d
c
f
e
b
ardumont added a comment.EditedDec 13 2018, 12:32 PM

Yes, but in regards to stuff fetching from the internet moving targets (pypi, npm), that's not caching anything.
I'm in the middle of building the swh-web one and that's not caching those steps.
Every single time, it does it all again.
That's slow.

In the end, i got a local problem which was the reason.
Now that it is solved, i'll revert this makefile change.

Also, note that for now, in the current master, there are only 3 containers.
We will have much more.

While that will still be true, let's just see if that poses a problem first!

ardumont edited the summary of this revision. (Show Details)Dec 13 2018, 11:04 PM