chore(trainer): improve get_job_logs docstring#438
chore(trainer): improve get_job_logs docstring#438pandu992003 wants to merge 1 commit intokubeflow:mainfrom
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
🎉 Welcome to the Kubeflow SDK! 🎉 Thanks for opening your first PR! We're happy to have you as part of our community 🚀 Here's what happens next:
Join the community:
Feel free to ask questions in the comments if you need any help or clarification! |
There was a problem hiding this comment.
Pull request overview
This PR updates the TrainerClient.get_job_logs docstring to better explain log retrieval and provide a streaming usage example for developers using the Kubeflow Trainer Python API.
Changes:
- Rewrote the
get_job_logsdocstring for improved clarity and formatting. - Added an inline usage example demonstrating real-time log streaming with
follow=True. - Updated parameter/return descriptions and added a note about empty output.
| """ | ||
| Retrieve logs from a specific step of a TrainJob. |
There was a problem hiding this comment.
The docstring starts with a blank line (the opening triple-quote is on its own line), which introduces an empty first line in generated docs/help(); align with the rest of this file by putting the one-line summary immediately after the opening """.
| """ | |
| Retrieve logs from a specific step of a TrainJob. | |
| """Retrieve logs from a specific step of a TrainJob. |
| name (str): Name of the TrainJob. | ||
| step (str): Step of the TrainJob to collect logs from | ||
| (e.g., dataset-initializer or node-0). | ||
| follow (bool, optional): If True, streams logs in real-time. | ||
| Defaults to False. |
There was a problem hiding this comment.
The Args section switches to "name (str):" style, but other TrainerClient methods in this file consistently use Google-style "name:" without repeating types; please keep the Args formatting consistent within the module (either revert here or update the surrounding methods for the same style).
| name (str): Name of the TrainJob. | |
| step (str): Step of the TrainJob to collect logs from | |
| (e.g., dataset-initializer or node-0). | |
| follow (bool, optional): If True, streams logs in real-time. | |
| Defaults to False. | |
| name: Name of the TrainJob. | |
| step: Step of the TrainJob to collect logs from | |
| (e.g., dataset-initializer or node-0). | |
| follow: If True, streams logs in real-time. Defaults to False. |
| Returns: | ||
| Iterator of log lines. | ||
| Iterator[str]: An iterator over log lines. | ||
|
|
There was a problem hiding this comment.
This docstring removed the Raises section, but get_job_logs can still raise (e.g., Kubernetes backend surfaces TimeoutError/RuntimeError via get_job, and LocalProcess backend raises ValueError when the job name is unknown); please document the relevant exceptions again so callers know what to handle.
| Raises: | |
| ValueError: The TrainJob with the given name does not exist (e.g., LocalProcess backend). | |
| TimeoutError: Timeout while retrieving the TrainJob or its logs (e.g., Kubernetes backend). | |
| RuntimeError: Failed to retrieve the TrainJob or its logs (e.g., Kubernetes backend). |
| Retrieve logs from a specific step of a TrainJob. | ||
|
|
||
| This method allows you to fetch logs either as a batch or stream them | ||
| in real-time using the `follow` parameter. |
There was a problem hiding this comment.
The docstring says logs are retrieved "from a specific step", but several backends treat the default step==node-0 as a sentinel to return logs from multiple steps/nodes (not just node-0); please clarify the default/step semantics so users know what output to expect.
|
Hi, thank you for reviewing! Please let me know if any changes are needed. |
Signed-off-by: pandu992003 <vigneshbangaru69@gmail.com>
78f1bbc to
1bc0368
Compare
|
Hi, thank you for reviewing! I’ve addressed all checks and updated the PR. Please let me know if any further improvements are needed. |
This PR improves the documentation for the
get_job_logsmethod in TrainerClient.Changes:
This helps developers better understand how to use the log retrieval functionality.