[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]

🤖 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.}

[cainpwrs]

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

[github-actions]

🤖 AI Code Example Review

This PR is in excellent shape. All previously raised items have been addressed in prior commits, and the current commit introduces no new defects. The code is well-structured, complete, and follows established repo patterns.

Detailed Review

1. [nit] Prior item (type annotation boto3.client on __init__ parameter in sesv2_wrapper.py, line 28) — still present in the current code. boto3.client is a factory function, not a valid type annotation. The correct annotation would be botocore.client.BaseClient or simply omitting it. This is a cosmetic nit consistent with a pre-existing pattern in the repo.

2. [nit] Prior item (hardcoded resource names in scenario_sesv2_search_filter_basics.py) — still present. Names like marketing-campaign-set and marketing@example.com could collide with existing resources. A comment near the constants explaining the expected naming convention, or a note that this is intentional for the scenario, would help. The integration test correctly uses UUID suffixes, so this inconsistency remains a minor nit.

3. [nit] Prior item (sesv2_metadata.yaml entries for sesv2_CreateConfigurationSet, sesv2_CreateTenant, sesv2_DeleteConfigurationSet, and sesv2_DeleteTenant have no description: field at the top level) — still present in the current diff. Other comparable entries include brief top-level description: fields. This is cosmetic.

4. [nit] The conftest.py uses sys.path.append(os.path.join(os.path.dirname(__file__), "..", "..", "..", "..")) with os.path.join (which is robust), unlike the prior concern about sys.path.append("../../../..") that was noted in an earlier review round. The path construction is now safe — this item is addressed.

5. [nit] The test_scenario_complete_integration integration test asserts only that run_scenario() does not raise. Adding even a minimal assertion (e.g., checking that the scenario printed the completion banner) would make the test more meaningful. This was noted in the prior review and remains a minor nit.

6. [policy] The sesv2_SearchFilterBasics metadata entry's synopsis_list uses plain text bullets rather than the &SESv2; entity reference style used elsewhere in the YAML file (e.g., compare with sesv2_Scenario_EmailAttachments). This is a documentation consistency policy note, not a code defect.

7. [nit] sesv2_wrapper.py's list_configuration_sets, list_email_identities, and list_tenants all use list() and dict() constructors instead of the more idiomatic [] and {} literals. This is a minor style inconsistency with common Python idioms, though it has no functional impact.

8. [nit] The requirements.txt bump to boto3>=1.43.105 and botocore>=1.43.105 lacks an inline comment explaining why this specific version is required (i.e., that it adds the Filter parameter to the SES v2 list operations). The scenario README mentions this, but a one-line comment in requirements.txt would help future maintainers.

9. Overall, the implementation is comprehensive: all prior blocking issues are resolved, cleanup is robust (each deletion is independently try/except wrapped), tests cover the scenario orchestration, wrapper methods, pagination, wait logic, and both unit and integration paths. The code is clean, well-commented, and closely follows repo patterns.

---

_{Reviewed at commit 15525d17d37a8ad06ff650b5aeb1210b49562264. 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.}