-
Notifications
You must be signed in to change notification settings - Fork 41
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
Update lock and requirements #2240
base: main
Are you sure you want to change the base?
Conversation
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
@@ -31,7 +31,7 @@ COPY --from=lightspeed-rag-content /rag/embeddings_model ./embeddings_model | |||
# (avoid accidental inclusion of local directories or env files or credentials) | |||
COPY runner.py requirements.txt ./ | |||
|
|||
RUN for a in 1 2 3 4 5; do pip3.11 install --no-cache-dir -r requirements.txt && break || sleep 15; done | |||
RUN pip3.11 install --no-cache-dir -r requirements.txt |
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.
Hi @onmete , why is this being removed? We need this for CI since it fails rather often without it.
We potentially need to include the returning of the error in case of failure in the last attempt but remove it all together is going to make CI flaky again.
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.
As we discussed before, there is something weird going on with this.
The build in this PR passed with the loop, but it is failing without it (same behavior I get locally).
The build error is not something a repeat will fix, yet it somehow passes with the loop.
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.
ok, I failed to understand previously that the build was passing in the loop, I thought it was failing but not returning any error. And that it stopped failing due to us not hitting the exact failing condition. That's why I suggested the || exit 1
at the end.
@onmete: The following tests failed, say
Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
Description
Type of change
Related Tickets & Documents
Checklist before requesting a review
Testing