[Serve] log deployment success message in serve.run before blocking #43718
Conversation
…oyment success message Signed-off-by: Gene Su <[email protected]>
|
Also tested manually and seeing "Deployed app successfully." is logged again |
|
@GeneDer I don't really want to add a callback to the public API (to reduce the API surface to maintain). It's probably useful to log this when folks call |
Partial bc there are 2 separate messages that we log "Deployed app successfully." and "Redeployed app successfully." and also the cli logger works different than our serve logger with some custom formatting and stuff. But yea let me simply by just always log a "Deployed app successfully." and drop the callback |
Got it, the original motivation makes sense. Let's just simplify and go w/ the single message. |
…e run Signed-off-by: Gene Su <[email protected]>
|
@edoakes this is how it looks now. It's no longer green and doesn't have the file name and line number. Also, noticed this is not using |
python/ray/serve/api.py
Outdated
|
|
||
| if blocking: | ||
| try: | ||
| while True: | ||
| # Block, letting Ray print logs to the terminal. | ||
| time.sleep(10) | ||
| except KeyboardInterrupt: | ||
| logger.info("Got KeyboardInterrupt, release blocking...") | ||
| logger.warning("Got KeyboardInterrupt, release blocking...") |
There was a problem hiding this comment.
What does "release blocking" mean here?
There was a problem hiding this comment.
Maybe better to say "Got KeyboardInterrupt, exiting"
python/ray/serve/api.py
Outdated
| @@ -573,14 +573,15 @@ def run( | |||
| route_prefix=route_prefix, | |||
| logging_config=logging_config, | |||
| ) | |||
| logger.warning("Deployed app successfully.") | |||
There was a problem hiding this comment.
| logger.warning("Deployed app successfully.") | |
| logger.warning(f"Deployed app '{name}' successfully.") |
There was a problem hiding this comment.
Due to this not using serve logger, info level won't log it to stdout 😅
Do you know why it doesn't have any log line formatting like |
Yes, bc it's not using serve's logger, it's doing |
|
Oh I see @GeneDer. What does it look like if we use the serve logger? I think a lot of the fields we usually set there aren't applicable on the driver... Maybe we should have an API logger (could maybe just pull in the |
|
This is how it looks if we just use serve logger (I didn't change back to info, but can easily do that) |
Signed-off-by: Gene Su <[email protected]>
|
@edoakes This is how it looks with the existing change |
GeneDer
changed the title
…ay-project#43718) In the previous PR, I accidentally blocked the cli logger from logging the deployment success message. log deployment success message in serve.run before blocking. --------- Signed-off-by: Gene Su <[email protected]>
Why are these changes needed?
In the previous PR, I accidentally blocked the cli logger from logging the deployment success message. log deployment success message in serve.run before blocking.
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.