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

[RHOAIENG-11349] - Missing requirements.txt for modelmesh-runtime-adpter component #65

Merged
merged 1 commit into from
Sep 19, 2024

Conversation

spolti
Copy link
Member

@spolti spolti commented Sep 2, 2024

chore: Adapt Dockerfile to work on Konflux.

Motivation

Modifications

Result

@openshift-ci openshift-ci bot added the approved label Sep 2, 2024
@spolti spolti force-pushed the RHOAIENG-11349 branch 2 times, most recently from c4ffae9 to 0d8771b Compare September 2, 2024 15:03
Dockerfile Outdated Show resolved Hide resolved
Dockerfile Outdated Show resolved Hide resolved
Dockerfile Show resolved Hide resolved
Dockerfile Show resolved Hide resolved
Dockerfile Outdated Show resolved Hide resolved
@spolti spolti force-pushed the RHOAIENG-11349 branch 3 times, most recently from c8ddd76 to 5fed933 Compare September 6, 2024 13:51
@spolti
Copy link
Member Author

spolti commented Sep 6, 2024

Moved red-hat-data-services#23 to here.

@spolti
Copy link
Member Author

spolti commented Sep 6, 2024

/test images

Dockerfile Show resolved Hide resolved
konflux-files/pyproject.toml Outdated Show resolved Hide resolved
@spolti spolti force-pushed the RHOAIENG-11349 branch 6 times, most recently from 5654610 to 7367985 Compare September 12, 2024 15:44
Copy link

@israel-hdez israel-hdez left a comment

Choose a reason for hiding this comment

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

One additional change...

Dockerfile Outdated
Comment on lines 172 to 178
pip install wheel && \
pip install grpcio && \
# pin to 3.10.0 to avoid error: libhdf5.so: cannot open shared object file: No such file or directory \
# if not version is set, it will install the 3.11.0 version which, seems that does not have the h5py dependencies \
# for arm yet.
pip install h5py==3.10.0 && \
pip install tensorflow
Copy link

@israel-hdez israel-hdez Sep 18, 2024

Choose a reason for hiding this comment

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

Replace all these pip install commands with a single pip install -r requirements.txt one and then I will be happy to approve.

Copy link

Choose a reason for hiding this comment

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

I agreed with @israel-hdez . However, if we replace this, we should update the requirements.txt file when the version changes.

Copy link
Member Author

Choose a reason for hiding this comment

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

right, I've forget to do this, I've added the requirements file inside the container but forgot to install from it

…pter component

chore:  Add the requirements.txt file and steps to generate it and correctly configure
        the Konflux build.
	Plus, adapt the Dockerfile to work properly witn Konflux.

Signed-off-by: Spolti <fspolti@redhat.com>
@spolti spolti changed the title [RHOAIENG-11349] - Missing requirements.txt for modelmesh-runtime-ada… [RHOAIENG-11349] - Missing requirements.txt for modelmesh-runtime-adpter component Sep 19, 2024
Copy link

@israel-hdez israel-hdez left a comment

Choose a reason for hiding this comment

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

/lgtm

Copy link

openshift-ci bot commented Sep 19, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: israel-hdez, spolti

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@israel-hdez
Copy link

@spolti will you cherry-pick to 0.12 branch? or is this only for main?

@openshift-merge-bot openshift-merge-bot bot merged commit 7d59320 into opendatahub-io:main Sep 19, 2024
10 checks passed
@spolti
Copy link
Member Author

spolti commented Sep 19, 2024

The plan is only for main, but we can cherry-pick later.

@spolti spolti deleted the RHOAIENG-11349 branch September 19, 2024 16:21
mholder6 pushed a commit to mholder6/modelmesh-runtime-adapter that referenced this pull request Oct 4, 2024
…pter component (opendatahub-io#65)

chore:  Add the requirements.txt file and steps to generate it and correctly configure
        the Konflux build.
	Plus, adapt the Dockerfile to work properly witn Konflux.

Signed-off-by: Spolti <fspolti@redhat.com>
mholder6 pushed a commit to mholder6/modelmesh-runtime-adapter that referenced this pull request Oct 4, 2024
…pter component (opendatahub-io#65)

chore:  Add the requirements.txt file and steps to generate it and correctly configure
        the Konflux build.
	Plus, adapt the Dockerfile to work properly witn Konflux.

Signed-off-by: Spolti <fspolti@redhat.com>
Signed-off-by: mholder6 <marholde@redhat.com>
israel-hdez pushed a commit that referenced this pull request Oct 4, 2024
…pter component (#65)

chore:  Add the requirements.txt file and steps to generate it and correctly configure
        the Konflux build.
	Plus, adapt the Dockerfile to work properly witn Konflux.

Signed-off-by: Spolti <fspolti@redhat.com>
Signed-off-by: mholder6 <marholde@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants