Conversation
Co-authored-by: Copilot <copilot@github.com>
Co-authored-by: Copilot <copilot@github.com>
There was a problem hiding this comment.
Code Review
This pull request updates the API routes to include versioning, adds new handlers for Pearson correlation and profile search, and refactors existing code. Feedback was provided regarding potential compatibility issues with older Scipy versions in the Pearson correlation implementation, a discrepancy between the profile search documentation and its actual implementation, and the need to maintain backward compatibility for renamed API endpoints.
There was a problem hiding this comment.
Pull request overview
This pull request refactors Pearson correlation and profile search endpoint logic out of app.py into dedicated handler functions under handlers/misc.py, and updates model deployment endpoints to use versioned routes.
Changes:
- Move
handle_pearsonranddo_profile_searchimplementations intotools/tdgpt/taosanalytics/handlers/misc.pyand update routing imports accordingly. - Change deployment routes to
/api/v1/deployand/api/v1/undeploy. - Expand the profile-search handler docstring with additional parameter/result documentation.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| tools/tdgpt/taosanalytics/handlers/misc.py | Adds Pearson and profile-search handlers (and expanded profile-search API docstring) to centralize miscellaneous business logic. |
| tools/tdgpt/taosanalytics/app.py | Updates routing to call the new handlers, switches deploy/undeploy endpoints to versioned paths, and standardizes logging via AppLogger. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: Copilot <copilot@github.com>
There was a problem hiding this comment.
Pull request overview
This PR refactors analytics endpoint logic by moving Pearson correlation and profile-search implementations out of the main Flask routing module into handlers/misc.py, while also updating deployment endpoints to use versioned API paths and expanding profile-search documentation.
Changes:
- Moved
handle_pearsonranddo_profile_searchimplementations fromapp.pyintohandlers/misc.py. - Updated deployment routes to
/api/v1/deployand/api/v1/undeploy. - Switched request logging to use
AppLoggerand expanded the profile-search docstring.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| tools/tdgpt/taosanalytics/handlers/misc.py | Adds Pearson correlation + profile-search handler functions and expands profile-search API documentation. |
| tools/tdgpt/taosanalytics/app.py | Updates imports, routes requests to the new handler functions, and changes deploy/undeploy endpoints to versioned paths. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR refactors analytics endpoint implementations by moving Pearson correlation and profile search logic out of app.py into handlers/misc.py, while also versioning model deployment endpoints under /api/v1/* and extending profile-search documentation.
Changes:
- Moved
handle_pearsonranddo_profile_searchbusiness logic intotools/tdgpt/taosanalytics/handlers/misc.pyand updated routing/logging accordingly. - Versioned deploy/undeploy endpoints to
/api/v1/deployand/api/v1/undeployand updated tests to match. - Implemented Prophet execution path in the dynamic forecast service (replacing the previous
NotImplementedError) and tightened config/log singleton typing/guards.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/tdgpt/tests/restful_api_test.py | Updates tests for versioned deploy/undeploy routes and adjusts profile-search assertions. |
| tools/tdgpt/taosanalytics/service_registry.py | Adds Prophet model execution logic for dynamic forecast services. |
| tools/tdgpt/taosanalytics/log.py | Adds typing/guards around the AppLogger singleton instance. |
| tools/tdgpt/taosanalytics/handlers/misc.py | Introduces dedicated handler functions for pearsonr + profile-search (moved from app.py). |
| tools/tdgpt/taosanalytics/conf.py | Hardens config module loading with explicit spec/loader checks and improves typing. |
| tools/tdgpt/taosanalytics/app.py | Switches routes to /api/v1/(un)deploy, delegates pearsonr/profile-search to handler module, and uses AppLogger consistently. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This pull request refactors the implementation of the Pearson correlation and profile search endpoints in the analytics service. The main logic for these features has been moved from
app.pyinto dedicated handler functions inhandlers/misc.py, improving code organization and maintainability. Additionally, the API endpoints for model deployment have been updated for better versioning, and the profile search API documentation has been expanded.Refactoring and Code Organization:
handle_pearsonranddo_profile_searchfromapp.pyintohandlers/misc.py, and updated imports and references accordingly. This centralizes business logic in handler modules and keeps the main app routing file cleaner.API Endpoint Improvements:
/api/v1/deployand/api/v1/undeployfor improved versioning and consistency.Profile Search API Documentation:
do_profile_searchwith more detailed parameter and result descriptions, including new options likewindow_size_step,window_sliding_step,exclude_contained, andexclude_source.Logging Consistency:
AppLoggerclass consistently instead of theapp_loggerinstance.Minor Cleanups:
osfromapp.py.app.py.These changes make the codebase more modular, easier to maintain, and improve the clarity and usability of the API.# Description
Checklist
Please check the items in the checklist if applicable.