-
Notifications
You must be signed in to change notification settings - Fork 1.4k
[confcom] Add a warning and path for default change for stdio #9203
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
rule | cmd_name | rule_message | suggest_message |
---|---|---|---|
confcom acifragmentgen | cmd confcom acifragmentgen added parameter enable_stdio |
||
confcom acipolicygen | cmd confcom acipolicygen added parameter enable_stdio |
Hi @DomAyre, |
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>
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds an --enable-stdio
flag to complement the existing --disable-stdio
flag and introduces a warning system to prepare users for a potential future default change from stdio enabled to disabled.
- Adds
--enable-stdio
argument option alongside existing--disable-stdio
- Implements warning logic when neither flag is specified to alert users about security implications
- Updates parameter handling to use a unified
stdio_enabled
variable instead of directly usingdisable_stdio
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
File | Description |
---|---|
src/confcom/azext_confcom/custom.py |
Adds stdio flag logic, warning messages, and unified parameter handling for both acipolicygen_confcom and acifragmentgen_confcom functions |
src/confcom/azext_confcom/_validators.py |
Adds mutual exclusion validation to prevent users from specifying both --enable-stdio and --disable-stdio flags |
src/confcom/azext_confcom/_params.py |
Defines the new --enable-stdio argument and updates parameter configurations for both commands |
|
Why
Addresses
How
--enable-stdio
arg to compliment--disable-stdio
and allow users to prepare for a default switchacipolicygen
andacifragmentgen
warn the user if they don't include either flag explaining the risks of leaving stdio enabled, warning the default may change in the future and explaining how to avoid that issue.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 azdev
required)python scripts/ci/test_index.py -q
locally? (pip install wheel==0.30.0
required)There is no functional change (only an extra arg which currently doesn't change default behaviour) no version bump is necessary