[rlhagerm]

This pull request adds the SESv2 Search and Filter example scenario.

---

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

[github-actions]

🤖 AI Code Example Review

This PR is a well-structured, complete implementation of the SESv2 Search and Filter scenario. It satisfies all specification requirements, includes unit and integration tests with proper stubbing infrastructure, handles errors correctly, and follows the repo's established patterns. A few minor nits and one policy observation are noted below.

Detailed Review

1. [policy] No unit tests cover the scenario orchestration class (SearchFilterBasicsScenario.run_scenario) directly with stubs — only the wrapper methods and an end-to-end integration test are present. Comparable scenarios (e.g., ControlTower) include a test_scenario_run.py that exercises the full orchestration path with mocked wrappers. This is not a blocking defect but should be tracked as a gap.

2. [nit] scenario_sesv2_search_filter_basics.py uses hardcoded resource names (marketing-campaign-set, marketing@example.com, etc.) that are unlikely to be unique if the scenario is run in an account that already has similarly named resources. The integration test correctly uses UUID suffixes; the runnable scenario could benefit from the same approach, or at minimum a comment explaining the expected naming convention.

3. [nit] In sesv2_wrapper.py, the type annotation boto3.client on the __init__ parameter is not a valid type — boto3.client is a factory function, not a type. The correct annotation is botocore.client.BaseClient or a string literal 'botocore.client.SESv2'. This mirrors a pre-existing pattern in the repo, so it is a nit rather than a blocking issue.

4. [nit] scenario_sesv2_search_filter_basics.py imports ClientError from botocore.exceptions but never raises or re-raises it directly — it is only used inside _setup_email_identities and _setup_tenants as an exception to catch. The import is correct and used, so this is just a note that the module-level exception handler in main() uses Exception which is broader than necessary.

5. [nit] The conftest.py uses a relative sys.path.append("../../../..") which is fragile and depends on the working directory at pytest invocation time. The comparable controltower example uses os.path.abspath to compute paths robustly. The sys.path.insert(0, os.path.join(os.path.dirname(__file__), "..")) line is fine; the sys.path.append("../../../..") line is the fragile one.

6. [nit] The _cleanup method in SearchFilterBasicsScenario does not swallow ClientError on individual resource deletions. If a single delete fails (e.g., resource was already deleted or never created due to an earlier error), the finally block will raise before cleaning up remaining resources. The wrapper's delete_* methods already swallow NotFoundException, but other errors (e.g., throttling) will propagate and abort cleanup mid-loop. Wrapping each deletion in a try/except inside _cleanup — as done in the integration test's fixture teardown — would make cleanup more robust.

7. [nit] requirements.txt bumps the minimum boto3/botocore version to 1.43.105 for new APIs (CreateTenant, ListTenants, DeleteTenant). The inline comment explaining why this bump is needed would help reviewers and future maintainers understand the constraint.

8. [nit] sesv2_metadata.yaml entries for sesv2_CreateConfigurationSet, sesv2_CreateTenant, sesv2_DeleteConfigurationSet, and sesv2_DeleteTenant have empty description: fields. Other comparable entries in the file provide brief descriptions; these should be filled in for consistency.

9. [nit] The test_scenario_complete_integration function patches scenario_sesv2_search_filter_basics.time.sleep to avoid the 15-second wait. The patch target is correct and the scenario's finally block ensures cleanup, but the test does not assert anything beyond 'no exception raised'. A minimal assertion (e.g., that the scenario printed the completion banner) would make the test more meaningful, though the current approach is acceptable.

10. [nit] sesv2_stubber.py imports from boto3 import client at the top level but only uses it as a type annotation. The idiomatic pattern in the repo's other stubbers (e.g., ses_stubber.py) imports boto3 directly and annotates with boto3.client. Either approach is fine, but consistency with the existing stubbers is preferred.

---

_{Reviewed at commit ad9f3ef7ebbf3d8602d05c84590baf28596dae69. This review was generated automatically using Amazon Bedrock. It compares your changes against existing examples and coding guidelines. Findings are labeled by severity (blocking / nit / policy) and reflect only the commit above. Please use your judgment — this is advisory, not authoritative.}

[github-actions]

🤖 [nit] If any individual delete_* call raises (e.g., a throttling error not caught by the wrapper), the loop aborts and subsequent resources are not cleaned up. Wrap each deletion in its own try/except to make cleanup unconditional, similar to the integration test fixture teardown pattern.

[github-actions]

🤖 [nit] These resource names are not unique across runs. If the scenario is run in an account that already has identically-named resources, creation will silently succeed for config sets (AlreadyExistsException is swallowed) and may produce confusing filtered results. Consider appending a short random suffix, or add a comment warning operators to use an account without these names.

[github-actions]

🤖 [nit] boto3.client is a factory function, not a type, so this annotation is technically incorrect. Prefer botocore.client.BaseClient or omit the annotation — this is consistent with several other wrappers in the repo that use the same pattern.

[github-actions]

🤖 [nit] sys.path.append("../../../..") is relative to the working directory, not to this file. This will break if pytest is invoked from a directory other than test/. Replace with sys.path.append(os.path.join(os.path.dirname(__file__), "../../../..")) for robustness.

[github-actions]

🤖 [nit] Consider adding a comment explaining why the minimum versions were bumped to 1.43.105 (e.g., # Required for CreateTenant, ListTenants, DeleteTenant API support). This makes the constraint self-documenting.

[github-actions]

🤖 [nit] from boto3 import client is only used as a type annotation. The rest of the repo's stubbers annotate with boto3.client after import boto3. Either import style is fine, but aligning with the existing pattern (e.g., ses_stubber.py) keeps the codebase consistent.

[github-actions]

🤖 AI Code Example Review

This PR is in excellent shape. All prior blocking and nit items from the previous review have been addressed: _cleanup now wraps each deletion in its own try/except (prior inline comment resolved), the scenario orchestration is fully tested via test_sesv2_search_filter_scenario.py (prior policy item resolved), and conftest.py uses os.path.join(os.path.dirname(__file__), ...) consistently (prior nit resolved). The remaining minor items from the previous review that are still technically present are noted below, but none are new or blocking.

Detailed Review

1. [nit] sesv2_metadata.yaml — the sesv2_CreateConfigurationSet, sesv2_CreateTenant, sesv2_DeleteConfigurationSet, and sesv2_DeleteTenant entries still have no top-level title:, title_abbrev:, or description: fields (only languages: and services: are present). Other action-level entries in the file typically include at least a brief description. This was flagged in the previous review and remains unchanged.

2. [nit] python/test_tools/sesv2_stubber.py line 9 — from boto3 import client is imported but used only as a type annotation on the __init__ parameter (sesv2_client: client). boto3.client is a factory function, not a type; the idiomatic annotation used elsewhere in the repo is boto3.client (as a string reference) or botocore.client.BaseClient. This was noted in the prior review and remains. It is purely cosmetic and does not affect runtime behavior.

3. [nit] scenario_sesv2_search_filter_basics.py — the module-level hardcoded resource names (marketing-campaign-set, marketing@example.com, customer-alpha, etc.) are not unique across concurrent runs or accounts that already have these resources. The integration test correctly uses UUID suffixes; a comment in the scenario explaining why these fixed names are intentional (or using a UUID suffix here too) would improve robustness. This was flagged previously and is still present.

4. [nit] requirements.txt bumps boto3/botocore to >=1.43.105. A short inline comment explaining why this minimum version is required (new CreateTenant, ListTenants, DeleteTenant APIs) would help future maintainers. This was noted before and is still absent.

5. [policy] The previous review noted the absence of a test_scenario_run.py-style test that exercises the full run_scenario() orchestration path with mocked wrappers (as seen in the ControlTower comparable). This has now been addressed by test_sesv2_search_filter_scenario.py, which includes test_run_scenario covering the complete orchestration. This item is resolved.

6. [nit] test_sesv2_search_filter_integ.py — test_scenario_complete_integration patches time.sleep to avoid the wait but asserts nothing beyond "no exception raised". A minimal assertion (e.g., checking the printed output contains the completion banner) would make the test more meaningful. This was noted previously and is still present, though the current approach is acceptable.

7. [nit] The overall code quality is high: clear step-by-step comments, consistent pagination patterns across all three list methods, robust cleanup that continues on individual failures, and well-structured stubber methods. The scenario, wrapper, unit tests, integration tests, stubber, and metadata are all present and consistent.

---

_{Reviewed at commit f1f5b598aeed385c28647efa78ba4a941f0fcbc3. This review was generated automatically using Amazon Bedrock. It compares your changes against existing examples and coding guidelines. Findings are labeled by severity (blocking / nit / policy) and reflect only the commit above. Please use your judgment — this is advisory, not authoritative.}

[github-actions]

🤖 [nit] from boto3 import client is used only as a type annotation. boto3.client is a factory function, not a type. Consider replacing with import boto3 and annotating as boto3.client (matching other stubbers in the repo), or remove the import and use a string annotation or botocore.client.BaseClient.

[github-actions]

🤖 [nit] Consider adding a comment explaining why 1.43.105 is the minimum: # Minimum version required for CreateTenant, ListTenants, and DeleteTenant APIs.

[cainpwrs]

Doc build: https://cainpwrs-doc-build-demo.w.tunnels.lab.aws.dev/PR-8228/