Add telemetry pipeline run ends #2377
Conversation
github-actions
bot
added
internal
|
Important Auto Review SkippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the To trigger a single review, invoke the WalkthroughThe recent updates focus on enhancing analytics and run tracking within the system. A new warning hint guides users on pipeline run states, and the introduction of a Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
|
@CodeRabbit review |
|
@coderabbitai review |
|
@coderabbitai review |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 1
Configuration used: .coderabbit.yaml
Files selected for processing (4)
- docs/book/user-guide/starter-guide/fetching-pipelines.md (1 hunks)
- src/zenml/analytics/enums.py (1 hunks)
- src/zenml/new/pipelines/pipeline.py (4 hunks)
- src/zenml/zen_stores/sql_zen_store.py (2 hunks)
Additional comments: 6
src/zenml/analytics/enums.py (1)
- 23-23: The addition of
RUN_PIPELINE_ENDEDto theAnalyticsEventenum class is correctly implemented and follows the naming convention of existing enum values. This change effectively extends the telemetry capabilities to include tracking the end of pipeline runs.docs/book/user-guide/starter-guide/fetching-pipelines.md (1)
- 104-112: The documentation update introduces a warning hint effectively, providing valuable information about the state of the pipeline run and how to obtain a refreshed version from the client. This addition enhances user understanding of pipeline run states and their management. However, ensure the code snippet is correctly formatted and clear to the readers.
src/zenml/new/pipelines/pipeline.py (3)
- 767-769: The method
_get_pipeline_analytics_metadatanow accepts an optionalrun_idparameter and includes it in the returned metadata if provided. This change aligns with the PR's objective to enhance telemetry capabilities by tracking the end of pipeline runs more accurately. Ensure that therun_idis being correctly utilized downstream to achieve the intended telemetry enhancements.- 762-772: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [742-769]
The changes made to the
_runmethod, including the early assignment ofrun_idand its inclusion in the_get_pipeline_analytics_metadatamethod, are logically sound and support the PR's objective of enhancing telemetry capabilities. However, ensure that the removal of the redundantrun_idinitialization does not affect the logic or functionality of the telemetry enhancements.
- 762-772: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [742-769]
The implementation of telemetry enhancements, including the handling of
run_idand its integration into analytics metadata, appears to be done with consideration for performance and maintainability. The changes are localized and do not introduce unnecessary complexity or security concerns. However, it's crucial to ensure that therun_idis handled securely throughout its lifecycle, especially if it's used in contexts where it could be exposed to external systems or users.src/zenml/zen_stores/sql_zen_store.py (1)
- 6695-6708: The implementation of analytics tracking within the
_update_pipeline_run_statusfunction is correctly placed and uses thetrack_handlercontext manager as expected. This ensures that the eventRUN_PIPELINE_ENDEDis captured whenever a pipeline run ends, which aligns with the PR's objective to enhance telemetry capabilities by tracking the end of pipeline runs. The metadata being captured, includingpipeline_run_id,status,num_steps,start_time, andend_time, is appropriate for monitoring and analytics purposes.
|
E2E template updates in |
| ```python | ||
| from zenml.client import Client | ||
|
|
||
| Client().get_pipeline_run(last_run.id) to get a refreshed version |
There was a problem hiding this comment.
Maybe we should also not call it last_run. This isn't actually the last run as someone might have created another one in the meantime, but this is exactly the run that was created by calling training_pipeline() in the script above.
| "pipeline_run_id": pipeline_run_id, | ||
| "status": new_status, | ||
| "num_steps": num_steps, | ||
| "start_time": pipeline_run.start_time.strftime( |
There was a problem hiding this comment.
just FYI, we don't send the start_time of the pipeline run in the other event, so there won't be any way to match that I assume. Not sure if we either want to add that in the other analytics event, or remove it here or maybe just send the duration here if that is interesting?
|
Quickstart template updates in |
* Addd another event * Added more context * Docstring * Auto-update of E2E template * Docs * python things * python things * python things * Auto-update of Starter template * Docstrings * Linting * Format --------- Co-authored-by: GitHub Actions <[email protected]>
Describe changes
I implemented an event that captures when a pipeline ends and also I added some docs info on pipelines
Pre-requisites
Please ensure you have done the following:
developand the open PR is targetingdevelop. If your branch wasn't based on develop read Contribution guide on rebasing branch to develop.Types of changes
Summary by CodeRabbit