[TEST PR] containerapp function get/liust, keys, invocations commands #9419
[TEST PR] containerapp function get/liust, keys, invocations commands #9419khushishah513 wants to merge 16 commits intoAzure:mainfrom
Conversation
resolving some of the comments on PR
resolving comments on PR
fixing style error
merging main
|
| rule | cmd_name | rule_message | suggest_message |
|---|---|---|---|
| containerapp debug | cmd containerapp debug added parameter debug_command |
||
| containerapp function | sub group containerapp function added |
|
Hi @khushishah513, |
|
Thank you for your contribution! We will review the pull request and get back to you soon. |
|
The git hooks are available for azure-cli and azure-cli-extensions repos. They could help you run required checks before creating the PR. Please sync the latest code with latest dev branch (for azure-cli) or main branch (for azure-cli-extensions). pip install azdev --upgrade
azdev setup -c <your azure-cli repo path> -r <your azure-cli-extensions repo path>
|
CodeGen Tools Feedback CollectionThank you for using our CodeGen tool. We value your feedback, and we would like to know how we can improve our product. Please take a few minutes to fill our codegen survey |
Release SuggestionsModule: containerapp
Notes
|
There was a problem hiding this comment.
Pull Request Overview
This pull request introduces comprehensive support for Azure Functions on Container Apps, including commands for managing functions, function keys, and invocation monitoring. The PR adds new commands for listing/showing functions, managing function keys, and retrieving invocation telemetry from Application Insights.
Key Changes
- Added new
containerapp functioncommands for listing and showing functions - Implemented function keys management (show, list, set operations)
- Added function invocations monitoring (summary and traces from App Insights)
- Enhanced the debug command to support executing commands inside containers
- Created new decorator classes for functions and function keys operations
Reviewed Changes
Copilot reviewed 15 out of 20 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| custom.py | Added command implementations for function operations and debug command enhancement |
| containerapp_functions_decorator.py | New decorator classes for function list/show/invocations operations |
| containerapp_function_keys_decorator.py | New decorator classes for function keys operations |
| containerapp_debug_command_decorator.py | New decorator for executing commands in containers |
| _validators.py | Added validation functions for function app operations |
| _utils.py | Added utility functions for replica selection and command execution |
| commands.py | Registered new command groups and transformers |
| test_containerapp_function.py | Comprehensive test suite for function operations |
| utils.py | Added helper for v1 environment preparation |
| # -------------------------------------------------------------------------------------------- | ||
|
|
||
| from subprocess import run | ||
| from time import sleep |
There was a problem hiding this comment.
The imports include both subprocess.run and time.sleep, but the code uses time.sleep throughout instead of the imported sleep. Either use the imported sleep or remove the from time import sleep import to be consistent.
| from time import sleep |
| container_app_name=name | ||
| ) | ||
|
|
||
| if revision_name and revision_name is not None: |
There was a problem hiding this comment.
The condition if revision_name and revision_name is not None: is redundant. If revision_name is truthy, it's already not None. Simplify to just if revision_name: for better readability.
|
|
||
|
|
||
| def get_random_replica(cmd, resource_group_name, container_app_name, revision_name): | ||
| logger.debug(f"Getting random replica for container app: name='{container_app_name}', resource_group='{resource_group_name}', revision='{revision_name}'") |
There was a problem hiding this comment.
The function is named get_random_replica but actually selects the replica with the latest creation time (line 867), not a random one. Consider renaming to get_latest_replica or get_running_replica to accurately reflect the behavior.
| container_app_name=name | ||
| ) | ||
|
|
||
| if revision_name and revision_name is not None: |
There was a problem hiding this comment.
The same redundant condition appears on line 148. Simplify to if revision_name: for consistency.
| import random | ||
|
|
There was a problem hiding this comment.
The random module is imported but never used in the code. Remove this unused import.
| import random |
| # Test: Show functions with non-existent revision should fail | ||
| with self.assertRaisesRegex(Exception, ".*"): |
There was a problem hiding this comment.
[nitpick] Missing blank line before comment (PEP 8 style). Add a blank line before line 142 to improve readability.
| debug_url = (f"wss://{proxy_api_url}/subscriptions/{sub}/resourceGroups/{resource_group_name}/" | ||
| f"containerApps/{name}/revisions/{revision}/replicas/{replica}/debug" | ||
| f"?targetContainer={container}") | ||
| return debug_url | ||
|
|
||
|
|
There was a problem hiding this comment.
[nitpick] The URL construction now splits the path across three lines with a trailing slash in the first line. This improves readability but introduces an unnecessary intermediate variable. Consider keeping the original inline return or adjusting line breaks to align with other URL constructions in the codebase.
| debug_url = (f"wss://{proxy_api_url}/subscriptions/{sub}/resourceGroups/{resource_group_name}/" | |
| f"containerApps/{name}/revisions/{revision}/replicas/{replica}/debug" | |
| f"?targetContainer={container}") | |
| return debug_url | |
| return (f"wss://{proxy_api_url}/subscriptions/{sub}/resourceGroups/{resource_group_name}/" | |
| f"containerApps/{name}/revisions/{revision}/replicas/{replica}/debug" | |
| f"?targetContainer={container}") |
This checklist is used to make sure that common guidelines for a pull request are followed.
Related command
General Guidelines
azdev style <YOUR_EXT>locally? (pip install azdevrequired)python scripts/ci/test_index.py -qlocally? (pip install wheel==0.30.0required)For new extensions:
About Extension Publish
There is a pipeline to automatically build, upload and publish extension wheels.
Once your pull request is merged into main branch, a new pull request will be created to update
src/index.jsonautomatically.You only need to update the version information in file setup.py and historical information in file HISTORY.rst in your PR but do not modify
src/index.json.