[Config]: standardize quoting style in Kubernetes manifests (#4353)#4359
Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
|
Hi @DejusDevspace , thanks for your help! Could you also run a search using 
|
|
Alright. I'll do that. Thank you |
There was a problem hiding this comment.
@CheyuWu @DejusDevspace Shouldn't this also be in double quotes? Same for the next line as well.
There was a problem hiding this comment.
Yes, I think it should be. I went through an earlier PR that quoted resource values in sample kubernetes manifests, and I believe this should be the same case.
I'll work on that, too. Thanks for pointing it out.
|
@DejusDevspace Would be better to clean up the commit history by squashing the commits into a single one. |
DejusDevspace
force-pushed
the
refactor/kubernetes-manifests
branch
from
74a6b11 to
0b7a897
Compare
|
@kash2104 Done. |
|
@DejusDevspace Just tick the test checkboxes in the PR description. |
|
@kash2104 Done. |
github-project-automation
bot
moved this from can be merged
to Done
in @Future-Outlier's kuberay project
|
LGTM. |
Future-Outlier
moved this from Done
to can be merged
in @Future-Outlier's kuberay project
…es-manifests Signed-off-by: Future-Outlier <eric901201@gmail.com>
|
@codex review |
|
Codex Review: Didn't find any major issues. Bravo. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
just solved the merge conflict |
| requests: | ||
| cpu: 300m | ||
| memory: 1G | ||
| cpu: "300m" |
There was a problem hiding this comment.
@Future-Outlier I think you reverted the latest changes? The latest change had this set to 1 CPU
There was a problem hiding this comment.
ah no, I think right now the PR is correct.
the latest change we updated the "head container"'s spec, not worker container's
There was a problem hiding this comment.
There was a problem hiding this comment.
line 69 here is for ray worker node container
There was a problem hiding this comment.
but I'm reviewing the diff here https://github.com/ray-project/kuberay/pull/4359/changes#diff-bc2e713c667545c5720faab290564c54ac21ef88ff924639b89c2f2fbf8de070
and it's not the same as #4702?
There was a problem hiding this comment.
- it's not the same as [RayService][e2e] fix flaky test TestAutoscalingRayService #4702
- [RayService][e2e] fix flaky test TestAutoscalingRayService #4702 changes the ray head container's resource spec
- this PR change the ray worker container's resource spec
- so we can merge this!
|
@codex review |
|
Codex Review: Didn't find any major issues. Breezy! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
github-project-automation
bot
moved this from can be merged
to Done
in @Future-Outlier's kuberay project
6 participants
Why are these changes needed?
This PR replaces remaining single-quoted string values in Kubernetes manifests with double quotes to align with the standardized quoting style introduced in #4353. No functional changes.
Related issue number
Closes #4353
Checks