-
Notifications
You must be signed in to change notification settings - Fork 5.8k
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
Set app_msg
to empty string by default
#35646
Set app_msg
to empty string by default
#35646
Conversation
Signed-off-by: Shreyas Krishnaswamy <[email protected]>
Signed-off-by: Shreyas Krishnaswamy <[email protected]>
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.
nit: app_msg
should be status_msg
?
LGTM. Leave approval to @zcin. Please manually test the workflow I described in the github issue before merging. |
Co-authored-by: Edward Oakes <[email protected]> Signed-off-by: shrekris-anyscale <[email protected]>
Signed-off-by: Shreyas Krishnaswamy <[email protected]>
Good catch, I changed it.
Sure, I manually tested the |
app_msg
to empty string by defaultapp_msg
to empty string by default
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!
The test failures are unrelated:
|
@edoakes @architkulkarni– This change is ready to merge. |
The ApplicationState currently updates its _status directly without necessarily updating the corresponding message. This causes stale messages to sometimes appear with updated statuses. This change sets the app_msg to an empty string by default, so the message is cleared or updated whenever the status is changed. Related issue number Closes ray-project#35508
The ApplicationState currently updates its _status directly without necessarily updating the corresponding message. This causes stale messages to sometimes appear with updated statuses. This change sets the app_msg to an empty string by default, so the message is cleared or updated whenever the status is changed. Related issue number Closes #35508
The ApplicationState currently updates its _status directly without necessarily updating the corresponding message. This causes stale messages to sometimes appear with updated statuses. This change sets the app_msg to an empty string by default, so the message is cleared or updated whenever the status is changed. Related issue number Closes ray-project#35508
The ApplicationState currently updates its _status directly without necessarily updating the corresponding message. This causes stale messages to sometimes appear with updated statuses. This change sets the app_msg to an empty string by default, so the message is cleared or updated whenever the status is changed. Related issue number Closes ray-project#35508 Signed-off-by: e428265 <[email protected]>
Why are these changes needed?
The
ApplicationState
currently updates its_status
directly without necessarily updating the corresponding message. This causes stale messages to sometimes appear with updated statuses. This change sets theapp_msg
to an empty string by default, so the message is cleared or updated whenever the status is changed.Related issue number
Closes #35508
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.test_update_app_deploy_failed
to make sure the error message is cleared.runtime_env
is originally invalid.