[serve] Use DeploymentID.__str__ to standardize log format.
#43708
Conversation
python/ray/serve/_private/common.py
Outdated
| @@ -28,6 +28,15 @@ class DeploymentID: | |||
| name: str | |||
| app_name: str = SERVE_DEFAULT_APP_NAME | |||
|
|
|||
| def __repr__(self): | |||
| s = f"Deployment(name='{self.name}'" | |||
| if not self.app_name or self.app_name == SERVE_DEFAULT_APP_NAME: | |||
There was a problem hiding this comment.
Non-blocker: I kinda feel it's fine to print "default" as the app name just in case if the user doesn't know what's the default value we use. Also, in the case when the user explicitly set it to be "default", it won't be omitted and cause confusion.
There was a problem hiding this comment.
Agreed, I also think it would be more clear if we print "default" as the app name. Also, now that 1.x deployments are fully deprecated, I think app_name should always be non-empty
There was a problem hiding this comment.
Ok, I will update it to include app='default'
python/ray/serve/_private/common.py
Outdated
| @@ -28,6 +28,15 @@ class DeploymentID: | |||
| name: str | |||
| app_name: str = SERVE_DEFAULT_APP_NAME | |||
|
|
|||
| def __repr__(self): | |||
| s = f"Deployment(name='{self.name}'" | |||
| if not self.app_name or self.app_name == SERVE_DEFAULT_APP_NAME: | |||
There was a problem hiding this comment.
Agreed, I also think it would be more clear if we print "default" as the app name. Also, now that 1.x deployments are fully deprecated, I think app_name should always be non-empty
edoakes
changed the title
DeploymentID.__repr__ to standardize log format.DeploymentID.__str__ to standardize log format.
edoakes
force-pushed
the
log-deployment-id
branch
from
c74d70a
to
3882d29
Compare
Why are these changes needed?
Uses a custom repr for
DeploymentIDinstead of hand-writing long strings describing each deployment.Before:
After:
I will do the same thing for replicas in: #43557
Related issue number
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.