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

Added support for local ARM architecture docker builds #32

Merged
merged 4 commits into from
Sep 5, 2024

Conversation

mluker
Copy link
Member

@mluker mluker commented Sep 4, 2024

  • Added support for ARM architecture when performing docker builds due to missing gcc libs.
  • Chatbot form updates
    • Added support for pressing enter to submit request
    • Made the input field required
    • Added the endpoint name to the output to show where the results originated from

Copy link
Member

@martinpeck martinpeck left a comment

Choose a reason for hiding this comment

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

Both parts of the PR look good to me (I'll let @stuartleeks give approval) but it might have been better to split these two things as they're quite separate.

The first part of this PR is the additional tools in Dockerfile to allow the Python package installation to work on ARM (because some packages don't have wheels for ARM). This looks reasonable. I've tested that this fails on a Ubuntu ARM without the PR changes, and that docker build works with the PR changes in place.

I'm also OK with the changes to the web page, but I actually think that the endpoint stuff (live vs simulator) is confusing and should be removed (see #30).

@mluker
Copy link
Member Author

mluker commented Sep 5, 2024

@martinpeck I've backed out the form changes and will PR that change separately without the endpoint output.

@martinpeck
Copy link
Member

Thanks for splitting into 2 PRs @mluker ! 👍

@stuartleeks
Copy link
Collaborator

Thanks @mluker!

Can you add some context around building for ARM? E.g. are you on a machine with an ARM chip and wanting to build and run the container image locally? Or are you targetting deployment to another environment with ARM chips?

@stuartleeks
Copy link
Collaborator

Or are you running locally on an ARM chip and trying to deploy to ACA? I just checked and realised that we don't currently specify the target platform when building the image to deploy to ACA!

@mluker
Copy link
Member Author

mluker commented Sep 5, 2024

@stuartleeks the context is running a docker build locally, docker build -t aoai-api-simulator . , on an M1 will fail due to gcc not being installed and not being able to build wheels for psutil.

image

@mluker mluker changed the title Added support for ARM architecture and some chat bot updates Added support for local ARM architecture docker builds Sep 5, 2024
@stuartleeks
Copy link
Collaborator

Ok, cool.

I've added #36 to track the image build for ACA deployment.
I've kicked off the validation build again and raised #33 to track resolving the failing test

@stuartleeks stuartleeks merged commit 60c9692 into microsoft:main Sep 5, 2024
2 checks passed
@mluker mluker deleted the mluker/arm-support-chat-updates branch September 5, 2024 15:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants