-
Notifications
You must be signed in to change notification settings - Fork 6k
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
[Dependencies] Remove kubernetes
requirement
#36333
[Dependencies] Remove kubernetes
requirement
#36333
Conversation
Signed-off-by: Archit Kulkarni <architkulkarni@users.noreply.github.com>
Signed-off-by: Archit Kulkarni <architkulkarni@users.noreply.github.com>
Signed-off-by: Archit Kulkarni <architkulkarni@users.noreply.github.com>
Signed-off-by: Archit Kulkarni <architkulkarni@users.noreply.github.com>
Signed-off-by: Archit Kulkarni <architkulkarni@users.noreply.github.com>
Signed-off-by: Archit Kulkarni <architkulkarni@users.noreply.github.com>
Signed-off-by: Archit Kulkarni <architkulkarni@users.noreply.github.com>
Assigning @iycheng as a codeowner from Ray Core |
# Source: | ||
# https://github.com/kubernetes-client/python/blob/master/kubernetes/utils/quantity.py | ||
from decimal import Decimal, InvalidOperation | ||
|
||
|
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.
This LGTM as this code has remained pretty much static in the upstream.
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.
LGTM!
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.
stamping dependency change
…ve-kubernetes-import
huggingface_text_classification broken on master, unrelated |
The kubernetes package is installed with ray["all"] but it's currently only used in two places: A helper function for converting units The GCS FT on K8s release test: ray/release/k8s_tests/run_gcs_ft_on_k8s.py Line 4 in 4070624 from kubernetes import client, config, watch This PR removes kubernetes from Ray's dependencies and removes it from the Ray docker image. This change is needed to reduce the overall package size of ray["all"]. For the helper function, we vendor it from the upstream. For the release test, kubernetes is installed separately: ray/release/k8s_tests/app_config.yaml Line 13 in 4070624 - kubernetes . I've also added it to release/requirements.txt to be safe. Related issue number Closes ray-project#36331 --------- Signed-off-by: Archit Kulkarni <architkulkarni@users.noreply.github.com> Signed-off-by: e428265 <arvind.chandramouli@lmco.com>
Why are these changes needed?
The
kubernetes
package is installed withray["all"]
but it's currently only used in two places:ray/release/k8s_tests/run_gcs_ft_on_k8s.py
Line 4 in 4070624
This PR removes
kubernetes
from Ray's dependencies and removes it from the Ray docker image. This change is needed to reduce the overall package size ofray["all"]
.For the helper function, we vendor it from the upstream.
For the release test,
kubernetes
is installed separately:ray/release/k8s_tests/app_config.yaml
Line 13 in 4070624
release/requirements.txt
to be safe.Related issue number
Closes #36331
Checks
git commit -s
) in this PR.scripts/format.sh
to lint the changes in this PR.method in Tune, I've added it in
doc/source/tune/api/
under thecorresponding
.rst
file.