-
Notifications
You must be signed in to change notification settings - Fork 325
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
docker image building for all of the docker images our integration tests require. #622
Conversation
…it checkouts to specified commits, add minio, bump the tini version, and fix fakesqs and localstack.
…lso, amd64 support.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(I'll review the Makefile tomorrow)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The parallel execution seems not to work (it stops halfway), due to the interleaved output it's not easy to see what is going wrong, however.
I've seen this in the logs:
no such manifest: docker.io/wireserver/dynamodb_local:0.0.7-386
Makefile:105: recipe for target 'manifest-create-dynamodb_local' failed
make: *** [manifest-create-dynamodb_local] Error 1
make: *** Waiting for unfinished jobs....
Currently trying without parallelism...
Running
Full logs of
|
Without parallelism:
There seems to still be an ordering issue here somewhere. |
I've managed to reproduce the failure you saw. not a critical one, but i'm looking into it. |
@julialongtin Do you have a good Makefile guide you can suggest? I understand the basics, but I'd love a resource where I can skim through advanced features :) A lot of these symbols and forms are tough to google. |
…es, along with race condition fixes.
Co-Authored-By: julialongtin <julia.longtin@gmail.com>
…ram when running integration tests.
This is some pretty complex stuff; props to you for getting it working, but I don't think I understand enough to give it a real review; maybe this is a good candidate for a show & tell? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The docs look great! I'll give the actual Makefile another look through on Monday 😄
…a, instead of the architecture specific image name.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- I left some TODOs in the docs you made.
- Could you follow the naming conventions for the mardown files (all small letters, dashes, not underscores)?
- I didn't finish reading the docs, but I skimmed through everything and established that it's more than good enough to make the Makefile maintainable whenever we need to get back to that.
- I have run the integration tests with your new docker-ephemeral deps, and failed because elastic search is not coming up.
- I am running
make build-all -j
now. Let's see if that helps.
building does not work either:
|
@julialongtin In the future I'd prefer if you could address outstanding comments and feedback before merging; there are a few comments on this PR which haven't been resolved or discussed yet. 😸 |
oops, that may have been my fault. i said "merge", and neither of us checked if everything was done. @julialongtin? |
I'll take a look when i hit the office. ;)
…On Tue, Mar 19, 2019 at 8:20 PM fisx ***@***.***> wrote:
oops, that may have been my fault. i said "merge", and neither of us
checked if everything was done. @julialongtin
<https://github.com/julialongtin>?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#622 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AQDAApfqCk9m9rl-0mQnZGDov1pX7yOcks5vYTiWgaJpZM4a5-da>
.
|
No description provided.