stain / jena-docker

Docker image for Apache Jena riot
Apache License 2.0
99 stars 86 forks source link

Fixes missing copy command for tdbloader2 and adds procps to image #50

Closed kuzeko closed 3 years ago

kuzeko commented 3 years ago

Addressing:

codextremist commented 3 years ago

Hi, any updates on why this PR hasn't been merged yet?

kuzeko commented 3 years ago

@codextremist I have the power to merge, but not sure if should overstep @stain

alexkreidler commented 3 years ago

Friendly ping @stain. Could we merge this soon?

@kuzeko I would recommend you wait maybe a few days for a response and then merge. If you check the contribution history, a good number of folks have merged PRs and added commits. And I'm sure @stain would rather have the project move forward than wait up and prevent folks from getting bug fixes, etc.

One curious thing is that the CI task seemed to have failed for this PR, but only on one command that greps the output for the word "Started", which hasn't changed since the last few builds. Mysterious...

Thanks to all for your work on this!

kuzeko commented 3 years ago

Hi @alexkreidler , ok I will merge now.

W.r.t. the CI I am guessing is the fact that the sleep was not "long enough". If that is the case, that part is better replaced with a call to a for loop that sleeps, queries the system to check if it is up and then fails completely after some time.

daxid commented 3 years ago

@alexkreidler @kuzeko I think Docker image is not building with this merge :

https://travis-ci.org/github/stain/jena-docker

This is a major problem leaving the all project basically unusable. Can you look into it please ?

b2m commented 3 years ago

W.r.t. the CI I am guessing is the fact that the sleep was not "long enough".

I think it is a little more complicated than that.

Comparing the output of the failed build with previous ones I noticed the difference in the output of docker logs fuseki.

  1. docker logs fuseki Shows the admin password part and the server information.
  2. docker logs fuseki | grep --quiet ^admin= Is successfull but shows the server info part as output despite getting piped to grep --quiet.
  3. docker logs fuseki | grep --quiet Started Is not successfull and shows the server info part as output despite getting piped to grep --quiet.

My hypothesis:

Possible solution: combine the two channels: docker logs fuseki 2>&1 | grep --quiet Started

kuzeko commented 3 years ago

Good catch @b2m

Here is a pull request to fix it: https://github.com/stain/jena-docker/pull/53

b2m commented 3 years ago

Here is a pull request to fix it:

53

Oh, now we can decide whether to merge #52 or #53 =)

kuzeko commented 3 years ago

AH! Nice one. Mine adds an additional curl call ( I wanted to remove the sleep 8 but then I didn't) I think we can go with #52

stain commented 3 years ago

Thanks @kuzeko, I've also invited @b2m and @alexkreider - feel free to merge without me, my email are particularly lossy at the moment.. :-/

kuzeko commented 3 years ago

Thanks @stain since you are around, do you think you can enable the tagging of the docker containers? See https://github.com/stain/jena-docker/issues/31