Skip to content

Conversation

@aemous
Copy link
Contributor

@aemous aemous commented Oct 22, 2025

Description of changes:

  • Usage of the new migration debug flag is tracked in the User-Agent header.
  • Add support for the pager output breaking change, with resolution determined by setting cli_pager config to the empty string.
  • An environment variable exists as an alternative to the --v2-debug command line option
  • A --no-v2-debug CLI option exists (boolean switch).
  • The text "AWS CLI v2 MIGRATION WARNING" is updated to "AWS CLI v2 UPGRADE WARNING"
  • Updated scalarparse runtime detection to only print warning if the user did not configure any setting for cli_timestamp_format.
  • Replaced cloudformation deploy's warning's reliance on the service response: output it even if the changeset is non-empty.
  • Updated warnings to give more guidance, especially retaining v1 behavior on v2
  • Updated the binary-params-base64 use case. It does not need to be prefixed with file://.
  • Replace use of uni print outside of customizations with print.

Description of tests:

  • Ran and passed all unit and functional tests.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@aemous aemous requested a review from a team October 22, 2025 20:20
@codecov-commenter
Copy link

codecov-commenter commented Oct 22, 2025

Codecov Report

❌ Patch coverage is 94.11765% with 2 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (debug-migration@db28bd8). Learn more about missing BASE report.

Files with missing lines Patch % Lines
awscli/clidriver.py 66.66% 1 Missing ⚠️
awscli/customizations/cloudformation/deploy.py 75.00% 1 Missing ⚠️
Additional details and impacted files
@@                Coverage Diff                 @@
##             debug-migration    #9810   +/-   ##
==================================================
  Coverage                   ?   93.32%           
==================================================
  Files                      ?      209           
  Lines                      ?    16909           
  Branches                   ?        0           
==================================================
  Hits                       ?    15780           
  Misses                     ?     1129           
  Partials                   ?        0           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

"type": "enhancement",
"category": "Migration",
"description": "Implement a ``--migrate-v2`` flag that detects breaking changes for AWS CLI v2 for entered commands."
"description": "Implement a ``--v2-debug`` flag that detects breaking changes for AWS CLI v2 for entered commands."
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add the env var here too.

Comment on lines 72 to 76
},
"no-v2-debug": {
"action": "store_true",
"dest": "no_v2_debug",
"help": "<p>Disable AWS CLI v2 migration assistance.</p>"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is for if you've set the environment variable, but want to opt-out for a single invocation?

If so, I don't think we need that at launch, can add it if folks ask for it. We have the slightly more verbose AWS_CLI_UPGRADE_DEBUG_MODE=false aws s3 ls as a workaround sooner.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that was the intended use, which is consistent with some other global args that have env vars, such as auto prompt mode and pager.

I'm not against removing it, will do this in next revision.

out_file=sys.stderr
)
if url_params and session.full_config.get('cli_follow_urlparam', True):
if url_params and session.get_scoped_config().get('cli_pager', None):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this still be cli_follow_urlparam?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch! The config we are checking should be updated to scoped config, but yes it should still be cli_follow_urlparam, looks like a copy-paste error.

Will revise.

Comment on lines 138 to 143
s3_config is not None and s3_config
.get(
'us_east_1_regional_endpoint',
'legacy'
) == 'legacy' and region in ('us-east-1', None)
):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not seeing the warning printed for aws s3api list-directory-buckets --region us-east-1 --v2-debug with profile


[default]
region = us-east-1
cli_history=enabled
retry_mode=standard
output = json

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch! Looks like I likely tested with high-level s3 commands for this particular breaking change, though it should apply to the modeled s3api commands too.

Will revise.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I addressed this, and also added 2 regression tests to test this (for s3 and s3api).

@aemous aemous requested a review from ashovlin December 4, 2025 17:24
@aemous
Copy link
Contributor Author

aemous commented Dec 4, 2025

An update is needed to detecting URL parameter usage (parameters starting with http:// or https://). The current solution has false positives. There is a blocklist of parameters that do not use the follow param feature, and any CLIArgument with attribute follow_urlparam = False do not use the feature. We do not check these two conditions in this PR currently.

To avoid bloating this PR with new changes, I'll follow up with this change in a future PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants