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.

Sign up for GitHub

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

Jump to bottom

Allow building Merlin with older versions of docker. #1064

Closed
wants to merge 8 commits into from
Closed

Allow building Merlin with older versions of docker. #1064

wants to merge 8 commits into from

Conversation

bashimao
Copy link
Contributor

@bashimao bashimao commented Sep 8, 2023

Modified images according to a trick discovered by @EmmaQiaoCh , which allows compiling the images on ARM machines with old versions of docker and versions that do not come with buildx.

@copy-pr-bot
Copy link

copy-pr-bot bot commented Sep 8, 2023

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@bashimao bashimao self-assigned this Sep 8, 2023
@bashimao bashimao added the bug Something isn't working label Sep 8, 2023
@github-actions
Copy link

github-actions bot commented Sep 8, 2023

Documentation preview

https://nvidia-merlin.github.io/Merlin/review/pr-1064

@bashimao bashimao force-pushed the fix_allow_building_with_old_docker branch from 189732c to 067244f Compare September 8, 2023 12:50
@bashimao
Copy link
Contributor Author

What this actually fixes:

  1. It fixes the perf_analyzer not being there on ARM64 systems.
  2. If suppresses the error where the Triton fil backend doesn't exist in the base image with older versions of docker.
  3. It fixes a bunch of typos.
  4. I removed the check that prevented installing python3-libnvinfer on ARM64 systems. That was a legacy issue that got resolved through fixes in the upstream image. Since the tritonserver switched to Ubuntu 22.04 as base image, libnvinfer is available.

@EmmaQiaoCh
Copy link
Contributor

/ok to test

@EmmaQiaoCh
Copy link
Contributor

/ok to test

@EmmaQiaoCh
Copy link
Contributor

The copied PR is merged

@EmmaQiaoCh EmmaQiaoCh closed this Sep 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@EmmaQiaoCh EmmaQiaoCh EmmaQiaoCh approved these changes

@jperez999 jperez999 Awaiting requested review from jperez999

Assignees

@bashimao bashimao

Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@bashimao @EmmaQiaoCh