Skip to content
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

fix(docker) Add platform to docker-compose command #5683

Conversation

firasomrane
Copy link
Contributor

@firasomrane firasomrane commented Aug 18, 2022

Fixes #5682
(+) changing docker -> docker_cli file name to follow cli files convention.

Tested executing python docker_cli.py (after adding if __name__ == "__main__": quickstart() at the end)
image

Also fixed docker/dev.sh file executing ./docker/dev.sh

image

With the fix it pulls mysql and neo4j images successfully with the right architecture.

Inspecting the image to verify the arch and looks good docker image inspect mysql_image_id

"Architecture": "amd64",
"Os": "linux",
"Size": 430582471,
"VirtualSize": 430582471,
...

@shirshanka shirshanka added community-contribution PR or Issue raised by member(s) of DataHub Community devops PR or Issue related to DataHub backend & deployment labels Aug 19, 2022
@firasomrane firasomrane force-pushed the fix/docker-use-amd64-arch-when-using-enumlator-on-m1 branch 2 times, most recently from fce6b79 to 9fc67d5 Compare August 20, 2022 11:31
@github-actions
Copy link

github-actions bot commented Aug 24, 2022

Unit Test Results (build & test)

571 tests  ±0   571 ✔️ ±0   14m 18s ⏱️ +4s
141 suites ±0       0 💤 ±0 
141 files   ±0       0 ±0 

Results for commit 8fd5f97. ± Comparison against base commit d5d8a38.

♻️ This comment has been updated with latest results.

@github-actions
Copy link

github-actions bot commented Aug 24, 2022

Unit Test Results (metadata ingestion)

       8 files  ±0         8 suites  ±0   54m 21s ⏱️ -23s
   709 tests ±0     706 ✔️ ±0  3 💤 ±0  0 ±0 
1 418 runs  ±0  1 412 ✔️ ±0  6 💤 ±0  0 ±0 

Results for commit 8fd5f97. ± Comparison against base commit d5d8a38.

♻️ This comment has been updated with latest results.

@firasomrane firasomrane reopened this Aug 24, 2022
Copy link
Collaborator

@pedro93 pedro93 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM overall, let's wait for CI to be green before running.
@firasomrane did you test this locally on a machine?
If yes what OS and if not could you please follow https://datahubproject.io/docs/developers#deploying-local-versions to deploy your local changes and deploy datahub.

Copy link
Contributor

@szalai1 szalai1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@firasomrane thanks for contributing this back. Could you add the same logic to docker/quickstart.sh as well?

@szalai1
Copy link
Contributor

szalai1 commented Aug 31, 2022

tested this locally on my M2 mac. Works fine.

@firasomrane
Copy link
Contributor Author

firasomrane commented Sep 1, 2022

@pedro93 Thanks for the review.
@szalai1 The docker/quickstart.sh file is added now, with other scripts using the docker-compose command

@firasomrane firasomrane force-pushed the fix/docker-use-amd64-arch-when-using-enumlator-on-m1 branch from 488eafa to 332445e Compare September 1, 2022 22:53
@firasomrane firasomrane force-pushed the fix/docker-use-amd64-arch-when-using-enumlator-on-m1 branch from 332445e to 53532bb Compare September 7, 2022 20:03
@firasomrane firasomrane force-pushed the fix/docker-use-amd64-arch-when-using-enumlator-on-m1 branch from 53532bb to 8376fbd Compare September 8, 2022 22:39
@anshbansal
Copy link
Collaborator

There are merge conflicts that need to be fixed

@anshbansal anshbansal self-assigned this Sep 12, 2022
@anshbansal anshbansal merged commit c606abd into datahub-project:master Sep 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community-contribution PR or Issue raised by member(s) of DataHub Community devops PR or Issue related to DataHub backend & deployment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Docker pulls images built with wrong platform when using Rosetta emulator with m1 chip with arm64 arch
6 participants