-
Notifications
You must be signed in to change notification settings - Fork 7.1k
[docs] Expose dashboard url when deploying on Yarn using Skein #57793
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
Conversation
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.
Code Review
This pull request introduces changes to the Ray documentation and configuration files to expose the Ray dashboard via Skein's web UI when deploying Ray on Yarn. The changes include adding a Python script (dashboard.py) to register the dashboard, modifying the ray-skein.yaml file to include the script and set the dashboard host, and updating the documentation to reflect these changes. I have identified a potential issue with the dashboard.py script and have provided a suggestion to improve its robustness.
f0745d4 to
a8499b3
Compare
ZacAttack
left a comment
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.
Let's add gemini's suggestion, but I think this looks good. Thanks!
066851e to
c3334ed
Compare
| @@ -0,0 +1,22 @@ | |||
| import skein | |||
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 seems to be the same file from doc_code/yard/dashboard.py -- is that intentional?
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.
Yes. I noticed there are same files under doc/cluster/doc_code/yarn/ and doc/yarn, such as example.py and ray-skein.yaml, so I add same changes under two folder.
| if len(sys.argv) < 2: | ||
| print("Usage: python dashboard.py <dashboard-address>") | ||
| sys.exit(1) |
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: use argparse instead of hand rolling this
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.
As there is only one required argument, I'd keep it simple as is.
80d8cea to
9118762
Compare
Signed-off-by: Zakelly <[email protected]>
9118762 to
0ecc1f8
Compare
|
Finally the lint got passed... |
|
Triggered full CI run: https://buildkite.com/ray-project/premerge/builds/52140 PR will auto-merge if it passes. Thanks for the contribution! |
|
Ah... CI failed. There seems some error related with the env: Is it possible to re-trigger this? @edoakes |
Re-triggered. For the future can just click "Update branch", which will merge master and re-trigger CI. |
|
CI passed but the auto-merge is disabled. Please merge this at your convenience, thanks a lot! @edoakes @ZacAttack |
…roject#57793) When deploying ray on Yarn using Skein, it's useful to expose the ray's dashboard via Skein's web ui. This PR shows how to expose that and update the related document. Signed-off-by: Zakelly <[email protected]> Co-authored-by: Edward Oakes <[email protected]> Signed-off-by: xgui <[email protected]>
…roject#57793) When deploying ray on Yarn using Skein, it's useful to expose the ray's dashboard via Skein's web ui. This PR shows how to expose that and update the related document. Signed-off-by: Zakelly <[email protected]> Co-authored-by: Edward Oakes <[email protected]>
…roject#57793) When deploying ray on Yarn using Skein, it's useful to expose the ray's dashboard via Skein's web ui. This PR shows how to expose that and update the related document. Signed-off-by: Zakelly <[email protected]> Co-authored-by: Edward Oakes <[email protected]> Signed-off-by: Aydin Abiar <[email protected]>
…roject#57793) When deploying ray on Yarn using Skein, it's useful to expose the ray's dashboard via Skein's web ui. This PR shows how to expose that and update the related document. Signed-off-by: Zakelly <[email protected]> Co-authored-by: Edward Oakes <[email protected]> Signed-off-by: Future-Outlier <[email protected]>
…roject#57793) When deploying ray on Yarn using Skein, it's useful to expose the ray's dashboard via Skein's web ui. This PR shows how to expose that and update the related document. Signed-off-by: Zakelly <[email protected]> Co-authored-by: Edward Oakes <[email protected]>
Description
When deploying ray on Yarn using Skein, it's useful to expose the ray's dashboard via Skein's web ui. This PR shows how to expose that and update the related document.
Types of change
Checklist
Does this PR introduce breaking changes?
Testing:
docs change only
Code Quality:
git commit -s)Documentation:
doc/source/(if applicable)Additional context
Here is an example of exposed entry on Skein's web ui: