[Dashboard] Display duration in milliseconds if under 1 second. #49064
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
Signed-off-by: Colton Woodruff <[email protected]>
coltwood93
changed the title
alanwguo
reviewed
Dec 5, 2024
Comment on lines
+30
to
+32
| if (duration.asSeconds() < 1) { | ||
| durationText = duration.format("SSS[ms]"); | ||
| } else if (duration.asMinutes() < 1) { |
There was a problem hiding this comment.
For numbers less than 1 minute, can we include milliseconds as well?
There was a problem hiding this comment.
also, can you add a test case in DurationText.component.test.tsx?
There was a problem hiding this comment.
For numbers less than 1 minute, can we include milliseconds as well?
Sure, I can add that! I'll add it momentarily. Do you mean to solely display milliseconds, or seconds and milliseconds?
also, can you add a test case in DurationText.component.test.tsx?
Yes, I'll take a look and see what I can come up with. Thanks for the review!
|
I mistakenly committed without signing, so I'm trying to figure out how to correct that... |
Signed-off-by: Colton Woodruff <[email protected]>
Signed-off-by: Colton Woodruff <[email protected]>
Signed-off-by: Chi-Sheng Liu <[email protected]>
Signed-off-by: hjiang <[email protected]>
Our usage for gtest and dependency is incorrect, in short, we shouldn't declare `@com_google_googletest//:gtest_main` as dependency and **meanwhile** have main function defined in the test. One weird behavior I noticed is, `SetUp` overriden function will be invoked twice somehow. Similar to #48892 Signed-off-by: hjiang <[email protected]>
Signed-off-by: hjiang <[email protected]>
## Why are these changes needed? PandaDataAccessor does not have function _munge_conflict ## Related issue number closes: #46275 ## Checks - [x] I've signed off every commit(by using the -s flag, i.e., `git commit -s`) in this PR. - [x] I've run `scripts/format.sh` to lint the changes in this PR. - [x] I've included any doc changes needed for https://docs.ray.io/en/master/. - [ ] I've added any new APIs to the API Reference. For example, if I added a method in Tune, I've added it in `doc/source/tune/api/` under the corresponding `.rst` file. - [x] I've made sure the tests are passing. Note that there might be a few flaky tests, see the recent failures at https://flakey-tests.ray.io/ - Testing Strategy - [ ] Unit tests - [ ] Release tests - [ ] This PR is not tested :( Signed-off-by: Kevin <[email protected]> Co-authored-by: Cheng Su <[email protected]>
Signed-off-by: kaihsun <[email protected]>
When we simplified the node visualization to exclude actor information, the node visualization ID could conflict with other nodes when all other information (e.g., operation type, index in the schedule) is the same. This PR fixes the issue by using a visualization ID that includes actor information, but the label continues to exclude actor information for conciseness.
This PR groups the Data release tests into sections. There aren't any actual changes to the tests themselves or their setups. --------- Signed-off-by: Balaji Veeramani <[email protected]>
closes: #48867 Signed-off-by: Superskyyy <[email protected]>
<!-- Thank you for your contribution! Please review https://github.com/ray-project/ray/blob/master/CONTRIBUTING.rst before opening a pull request. --> <!-- Please add a reviewer to the assignee section when you create a PR. If you don't have the access to it, we will shortly find a reviewer and assign them to your PR. --> ## Why are these changes needed? Currently there are some docs in https://ray-project.github.io/kuberay/deploy/installation/ but they are not complete and didn't work for me, instead add working instructions and also show how to delete stuff. This might be suboptimal -- open to whether we want to merge it. If we do, we need to figure out how to name the cluster `raycluster-kuberay` instead of `raycluster-sample` so the rest of the instructions still work. ## Related issue number <!-- For example: "Closes #1234" --> ## Checks - [ ] I've signed off every commit(by using the -s flag, i.e., `git commit -s`) in this PR. - [ ] I've run `scripts/format.sh` to lint the changes in this PR. - [ ] I've included any doc changes needed for https://docs.ray.io/en/master/. - [ ] I've added any new APIs to the API Reference. For example, if I added a method in Tune, I've added it in `doc/source/tune/api/` under the corresponding `.rst` file. - [ ] I've made sure the tests are passing. Note that there might be a few flaky tests, see the recent failures at https://flakey-tests.ray.io/ - Testing Strategy - [ ] Unit tests - [ ] Release tests - [ ] This PR is not tested :(
Cleanup shared pointer and use unique pointer for clear memory ownership and less error prune. --------- Signed-off-by: hjiang <[email protected]>
Remove a useless function: - `BroadcastRaySyncMessage` is just a wrapper around `BroadcastMessage`, doing nothing else - The comment is confusing also, I don't see anything dangerous which breaks the state machine Signed-off-by: hjiang <[email protected]>
…ch the function signature" (#47773) bind doesn't check if the arg matches the signature. Fortunately, we already have the logic in .remote that checks it, and we just use the same mechanism. This also provides very nice error message;
## Why are these changes needed?
When users using multiple datasets and want to set different DataContext
configurations.
The recommended way is to set `DataContext.get_current()` before
creating a Dataset. The DataContext is supposed to be captured and
sealed by a Dataset when it's created. For example:
```python
import ray
context = ray.data.DataContext.get_current()
context.target_max_block_size = 100 * 1024 ** 2
ds1 = ray.data.range(1)
context.target_max_block_size = 1 * 1024 ** 2
ds2 = ray.data.range(1)
# ds1's target_max_block_size will be 100MB
ds1.take_all()
# ds2's target_max_block_size will be 1MB
ds2.take_all()
```
However in Ray Data internal code, `DataContext.get_current()` has been
widely used in an incorrect way. This PR fixes most outstanding issues
(but not all), by explicitly passing around the captured DataContext
object as an argument to each component.
## Related issue number
#41573
---------
Signed-off-by: Hao Chen <[email protected]>
Signed-off-by: hjiang <[email protected]>
Signed-off-by: hjiang <[email protected]>
coltwood93
requested review from
pcmoritz,
kevin85421,
a team,
sven1977,
simonsays1980 and
hongchaodeng
as code owners
|
I think I just blew up this PR. Going to close and open a fresh one to resolve issue #49010 |
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?
On the Ray Dashboard, a duration less than 1 second is displayed as "0s", which is not accurate.
Related issue number
Closes #49010
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.