[core] fix worker launch time formatting #43516
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ruisearch42
reviewed
Jun 11, 2024
python/ray/util/state/common.py
Outdated
| @@ -93,6 +93,15 @@ def timestamp(x: float): | |||
| """Converts miliseconds to a datetime object.""" | |||
| return str(datetime.datetime.fromtimestamp(x / 1000)) | |||
|
|
|||
| def timestamp_or_empty(x: int): | |||
There was a problem hiding this comment.
Does it make sense to unify timestamp_or_empty and timestamp into a single method that can take either float or int, valid values and invalid values?
This way user does not need to decide which exact method to call.
|
@hongchaodeng this PR still relevant; couple feedback points waiting on you. |
anyscalesam
added
triage
hongchaodeng
force-pushed
the
fix-launchtime
branch
from
bcce315
to
37f9fbb
Compare
jjyao
added
go
Signed-off-by: hongchaodeng <hongchaodeng1@gmail.com>
hongchaodeng
force-pushed
the
fix-launchtime
branch
from
37f9fbb
to
8e55004
Compare
jjyao
reviewed
Jul 17, 2024
python/ray/util/state/common.py
Outdated
Comment on lines
98
to
99
| if x == -1: | ||
| return "" |
There was a problem hiding this comment.
I think we should do it on the caller side as the caller knows what special value can be so that this method doesn't need to handle -1, None or some other special values
hongchaodeng
force-pushed
the
fix-launchtime
branch
from
e95b6a8
to
fcb37a4
Compare
hongchaodeng
assigned jjyao, rynewang and hongchaodeng and unassigned jjyao, rynewang and anyscalesam
rynewang
approved these changes
Jul 18, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Why are these changes needed?
Timestamp was not set for when triggering the launcher, resulting in default timestamp being set as (int) -1 == (unsigned) 18446744073709551615 (2^64-1)
Related issue number
Closes #35130
Checks
git commit -s) in this PR.scripts/format.shto lint the changes in this PR.method in Tune, I've added it in
doc/source/tune/api/under thecorresponding
.rstfile.