Skip to content
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

[v2] Docs rewrite rules #9325

Merged
merged 27 commits into from
Mar 10, 2025
Merged

[v2] Docs rewrite rules #9325

merged 27 commits into from
Mar 10, 2025

Conversation

aemous
Copy link
Contributor

@aemous aemous commented Feb 26, 2025

Description of changes:

  • Created a custom Sphinx extension that generates a package.redirects.conf file containing Apache redirect rules for each documented service operation supported by the AWS CLI v2.
  • Added the extension to the Sphinx conf.py file's extensions list so that it is invoked during executions of Sphinx build.

Description of tests:

  • Manually generated the docs locally following our documentation (i.e. via make all).
    • Verified a package.redirects.conf file is generated to our build output folder /doc/build/html/package.redirects.conf.
    • Verified that each RewriteRule in the generated file should be there, by verifying the exact same rule is generated in the corresponding file for AWS CLI v1 (using the test script below).
    • Verified that no RewriteRules that should be generated are excluded, by verifying that each rule generated for AWS CLI v1 is present in the generated file (using the test script below).
  • Ran and passed all of our test suites (unit, functional, integration, etc.).
  • Ran the AWS CLI v2 build CI/CD pipeline and verified that the generated documentation artifact contains the generated package.redirects.conf in its root.

Test script used for verifying correctness of the generated file's contents:

import os

if __name__ == '__main__':
    out_path = os.path.join(os.path.abspath('.'), 'package.redirects.conf') # path to CLI v2 generated file
    out_path2 = os.path.join(os.path.abspath('.'), 'package.redirects.conf.comp') # path to CLI v1 file (for comparison)

    def produce_lines(file_path, index):
        with open(file_path, 'r') as out_file:
            new_lines = []
            line_num = 0
            for line in out_file:
                line_num += 1
                if line_num < 3:
                    continue
                str = line[index:].split()[0]
                new_lines.append(str)
        return new_lines

    lines1 = produce_lines(out_path, 24) # remove cliv2-specific prefix from string for comparison
    lines2 = produce_lines(out_path2, 27) # remove cliv1-specific prefix from string for comparison
    print(f'Number of CLIv2 rewrite rules: {len(lines1)}')
    print(f'Number of CLIv1 rewrite rules: {len(lines2)}')
    lines1_cpy = list(lines1)
    lines2_cpy = list(lines2)

    for line in lines1:
        if line in lines2_cpy:
            lines2_cpy.remove(line)

    for line in lines2:
        if line in lines1_cpy:
            lines1_cpy.remove(line)

    print(f'RewriteRules in cliv1 not in cliv2: {lines2_cpy}')
    print(f'RewriteRules in cliv2 not in cliv1: {lines1_cpy}')

After generating the redirects file via make all using the extension added in this PR, I observed the following output from running this script:

Number of CLIv2 rewrite rules: 17036
Number of CLIv1 rewrite rules: 17045
RewriteRules in cliv1 not in cliv2: ['pinpoint-sms-voice-2018-09-05/CreateConfigurationSet$', 'pinpoint-sms-voice-2018-09-05/CreateConfigurationSetEventDestination$', 'pinpoint-sms-voice-2018-09-05/DeleteConfigurationSet$', 'pinpoint-sms-voice-2018-09-05/DeleteConfigurationSetEventDestination$', 'pinpoint-sms-voice-2018-09-05/GetConfigurationSetEventDestinations$', 'pinpoint-sms-voice-2018-09-05/ListConfigurationSets$', 'pinpoint-sms-voice-2018-09-05/SendVoiceMessage$', 'pinpoint-sms-voice-2018-09-05/UpdateConfigurationSetEventDestination$', 'pinpoint-sms-voice-2018-09-05/(.*)$']
RewriteRules in cliv2 not in cliv1: []
  • 8 of the 9 RewriteRules being flagged as being in v1 that are NOT in v2 can be explained by the fact that cliv1 generates duplicate rules of all operations under the service pinpoint-sms-voice, since it is a copy of the service sms-voice. These 8 are all of the duplicate modeled operations.
  • The remaining RewriteRule being flagged as being in v1 that is NOT in v2 is pinpoint-sms-voice-2018-09-05/ListConfigurationSets$. This can be explained by the fact that the CLI v2 does not currently support this operation as of testing.

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 review from aws-sdk-python-automation and a team February 26, 2025 20:22
@aemous aemous marked this pull request as ready for review February 26, 2025 20:56
Copy link
Member

@kdaily kdaily left a comment

Choose a reason for hiding this comment

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

Looks good! Also had the same comment about the absolute URI previously used, but that was taken care of.

I think you could run the pre-commit hook on these and the issues I saw would get taken care of, maybe some more still.

@kdaily
Copy link
Member

kdaily commented Mar 7, 2025

Since you already have committed, you can do this to run pre-commit on the files changed from the commit before your first change (check the commit hash to confirm):

pre-commit run --from-ref 396f778b50309981ed78505c95f11f90db274e1e --to-ref HEAD

Reference from https://pre-commit.com/ under "Some example useful invocations".

aemous and others added 2 commits March 10, 2025 10:31
Co-authored-by: Kenneth Daily <kdaily@amazon.com>
Copy link
Member

@kdaily kdaily left a comment

Choose a reason for hiding this comment

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

LGTM

@aemous aemous merged commit b9943c9 into aws:v2 Mar 10, 2025
45 checks passed
@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 0.00%. Comparing base (af7561b) to head (839af66).
Report is 14 commits behind head on v2.

Additional details and impacted files
@@    Coverage Diff     @@
##   v2   #9325   +/-   ##
==========================
==========================

☔ 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.

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.

7 participants