Replace partition by rpartition for parsing app_id #523
Conversation
facebook-github-bot
added
CLA Signed
|
This pull request was exported from Phabricator. Differential Revision: D37187555 |
Codecov Report
@@ Coverage Diff @@
## main #523 +/- ##
==========================================
+ Coverage 94.78% 94.79% +0.01%
==========================================
Files 65 65
Lines 3932 3942 +10
==========================================
+ Hits 3727 3737 +10
Misses 205 205
Continue to review full report at Codecov.
|
Summary: To parse a address like `"addr-of-cluster-app_id"`, the old code will fail since it takes `"addr"` as the address and `"of-cluster-app_id"` as the app id. After replace `partition` by `rpartition`, it should work as intended. Differential Revision: D37187555 Pulled By: d4l3k fbshipit-source-id: 8ce3422481ec060db54a9bf4c508dbe668bbd3d9
|
@d4l3k has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
| client = JobSubmissionClient(f"http://{addr}") | ||
| client.stop_job(app_id) | ||
|
|
||
| def _get_job_status(self, app_id: str) -> JobStatus: |
There was a problem hiding this comment.
It would be good to have explicit concepts that expresses app_id and maybe_app_id_address.
There was a problem hiding this comment.
yeah, long term I'm not very happy with how we're adding the addr in here -- I think we might need to do some config refactors on this so there's a way to set a "scheduler" config for non submit methods. Most other schedulers pull context info like this from the environment variables but ray doesn't seem to do that
Summary:
To parse a address like
"addr-of-cluster-app_id", the old code will fail since it takes"addr"as the address and"of-cluster-app_id"as the app id.After replace
partitionbyrpartition, it should work as intended.Differential Revision: D37187555
Pulled By: d4l3k