[Python] Add CloudWatch Logs Basics scenario + Hello#7976
Conversation
scmacdon
added
auto-generated
Python
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
🤖 AI Code Example Review
FAIL. The PR is critically incomplete and broken: the main scenario file (scenario_cloudwatch_logs_basics.py) referenced by both the test file and the metadata is entirely absent from the PR, meaning neither the scenario nor the integration tests can execute. Additionally, several structural and quality issues prevent this from meeting AWS SDK example standards.
Detailed Review
-
[BLOCKING] Missing scenario file: The test file
cloudwatch-logs_basics_scenario.pyimportsfrom scenario_cloudwatch_logs_basics import CloudWatchLogsScenario, and the metadata references the snippet tagpython.example_code.cloudwatch-logs.CloudWatchLogsScenario, but no such file exists anywhere in the PR. This is the core deliverable of the PR and it is simply not present. Nothing can run without it. -
[BLOCKING] Wrong file placed in scenarios directory: The file
python/example_code/cloudwatch-logs/scenarios/cloudwatch-logs_basics_scenario.pycontains integration test code (withimport pytest,@pytest.mark.integ, etc.), not a scenario. It appears the test file was accidentally placed in the scenarios directory. It should live undertest/and the actual scenario implementation should be inscenarios/. -
[BLOCKING] Missing test directory and unit tests: Per AWS Python example standards, there should be a
test/directory with unit tests using stubbers (not just integration tests). The comparable examples in the knowledge base usestubber_factory.pyand include aCloudWatchLogsStubber. Only integration tests are present, which cannot be run in CI without real AWS credentials and will incur charges. -
[BLOCKING] Missing README.md: All comparable premium examples include a
README.mdin the example directory. This file is completely absent from the PR. -
[STRUCTURAL] File naming inconsistency: The PR description mentions
hello/hello_cloudwatch-logs.pybut the actual file is atcloudwatch_logs_hello.py(flat, not in ahello/subdirectory, and using underscores not hyphens). The metadatagithubpath points topython/example_code/cloudwatch-logsfor the Hello example but the standard pattern places Hello examples in ahello/subdirectory. -
[STRUCTURAL] Metadata YAML destructively overwrites existing entries: The diff removes all pre-existing entries for other languages (.NET, Java, JavaScript, Kotlin, C++, SAP ABAP) from
cloudwatch-logs_metadata.yaml. This is extremely destructive — it deletes working examples forAssociateKmsKey,CancelExportTask,CreateExportTask,DescribeLogStreams,DescribeExportTasks,DescribeSubscriptionFilters,DeleteSubscriptionFilter,PutSubscriptionFilter,GetLogEvents, and the BigQuery scenario. The PR should only add Python entries to existing action blocks, not replace them. -
[QUALITY] Unused import in wrapper:
from datetime import datetime, timezoneis imported incloudwatch-logs_wrapper.pybut never used. This will trigger pylint warnings and fails the linting standard. -
[QUALITY]
get_query_resultspolling lacks a timeout/max-retries guard: Thewhile Trueloop inget_query_resultspolls indefinitely with only a 1-second sleep. If the query stays inRunningorScheduledstate for an extended period (e.g., due to service issues), this will hang forever. A maximum wait time or retry count should be enforced. -
[QUALITY]
start_live_tailevent_stream not closed on exception: If aClientErroror unexpected exception occurs afterevent_streamis opened inside the for-loop, the stream is not explicitly closed in afinallyblock. This can leak resources. -
[QUALITY]
requirements.txtmissing newline at EOF andpytestbelongs in a dev/test dependency:pytest>=7.0.0is a test-only dependency. Standard practice is to separate it (e.g.,pytestin arequirements-test.txtor clearly comment it). Also, the file is missing a trailing newline which is a minor style issue but noted in the diff.
This review was generated automatically using Amazon Bedrock. It compares your changes against existing examples and coding guidelines. Please use your judgment — this is advisory, not authoritative.
| @@ -0,0 +1,144 @@ | |||
| # Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. | |||
There was a problem hiding this comment.
🤖 This file contains integration test code (pytest fixtures, @pytest.mark.integ decorators) but is placed in the scenarios/ directory. It should be moved to test/test_cloudwatch-logs_integ.py. The actual scenario implementation (CloudWatchLogsScenario class with run_scenario()) is missing entirely and needs to be created.
| sys.path.insert(0, os.path.abspath(os.path.join(os.path.dirname(__file__), ".."))) | ||
|
|
||
| from cloudwatch_logs_wrapper import CloudWatchLogsWrapper | ||
| from scenario_cloudwatch_logs_basics import CloudWatchLogsScenario |
There was a problem hiding this comment.
🤖 from scenario_cloudwatch_logs_basics import CloudWatchLogsScenario — this import will fail at runtime because scenario_cloudwatch_logs_basics.py does not exist in the PR. The scenario file must be created.
| """ | ||
|
|
||
| import logging | ||
| import time |
There was a problem hiding this comment.
🤖 from datetime import datetime, timezone is imported but never used anywhere in the file. Remove this import to avoid pylint W0611 (unused-import) errors.
| query_string, | ||
| ) | ||
| raise | ||
|
|
There was a problem hiding this comment.
🤖 The while True polling loop has no timeout or maximum retry limit. If the Insights query remains in Running/Scheduled state indefinitely (e.g., service issue), this will hang forever. Add a max_attempts parameter or a wall-clock timeout (e.g., 5 minutes) and raise an error if exceeded.
| "Live Tail session started (ID: %s).", | ||
| session_start.get("sessionId", "unknown"), | ||
| ) | ||
| elif "sessionUpdate" in event: |
There was a problem hiding this comment.
🤖 The event_stream is not closed in a finally block. If an unhandled exception occurs during event iteration, the streaming connection will leak. Wrap the iteration in a try/finally that calls event_stream.close().
| @@ -1,398 +1,205 @@ | |||
| # zexi 0.2.0 | |||
| cloudwatch-logs_AssociateKmsKey: | |||
| cloudwatch-logs_Hello: | |||
There was a problem hiding this comment.
🤖 This diff completely removes existing entries for other languages (.NET, Java, JavaScript, Kotlin, C++, SAP ABAP) including actions like AssociateKmsKey, CancelExportTask, DescribeSubscriptionFilters, etc. Python entries should be ADDED to the existing action blocks, not used to replace all other languages. This will break documentation for all other SDK languages.
| @@ -0,0 +1,2 @@ | |||
| boto3>=1.26.0 | |||
| pytest>=7.0.0 No newline at end of file | |||
There was a problem hiding this comment.
🤖 Missing newline at end of file. More importantly, pytest is a test-only dependency. Consider separating it into a requirements-test.txt or at minimum adding a comment like # Test dependencies. Also botocore should be listed explicitly since it is a direct dependency in error handling.
|
|
||
|
|
||
| # snippet-start:[python.example_code.cloudwatch-logs.Hello] | ||
| def hello_cloudwatch_logs(): |
There was a problem hiding this comment.
🤖 Per AWS example standards, the Hello example should accept the boto3 client as a parameter (e.g., def hello_cloudwatch_logs(logs_client)) rather than creating it internally. This makes the function testable with stubbers and follows the pattern seen in comparable examples like hello_medical_imaging(medical_imaging_client).
| """ | ||
| try: | ||
| params = dict() | ||
| params["logGroupName"] = log_group_name |
There was a problem hiding this comment.
🤖 Style nit: params = dict() throughout the file should use params = {} (literal syntax), and list() should use []. The dict/list literal style is more idiomatic Python and is preferred per PEP 8 and the comparable premium examples.
| # Mock user inputs for the scenario: | ||
| # Press Enter (multiple times for continue prompts), then "y" for cleanup | ||
| answers = [ | ||
| "", # Press Enter to start |
There was a problem hiding this comment.
🤖 input_mocker is referenced as a pytest fixture but there is no conftest.py in the PR that defines it. Without this fixture definition, the test will fail with a fixture-not-found error. A conftest.py providing input_mocker (and likely stubber) must be added.
1 participant
Summary
This PR adds Python code examples for CloudWatch Logs generated by the CodeLoom
automated pipeline.
Files Added
python/example_code/cloudwatch-logs/hello/hello_cloudwatch-logs.py— Hello World examplepython/example_code/cloudwatch-logs/scenarios/cloudwatch-logs_basics_scenario.py— Scenariopython/example_code/cloudwatch-logs/cloudwatch-logs_wrapper.py— Service wrapper classpython/example_code/cloudwatch-logs/test/test_cloudwatch-logs_integ.py— Integration testspython/example_code/cloudwatch-logs/requirements.txt— Dependencies.doc_gen/metadata/cloudwatch-logs_metadata.yaml— Metadata for doc generationOperations Covered
(See specification for full list)
Generation Details
amazon-cloudwatch-logs-spec-20260710-141825amazon-cloudwatch-logs-codegen-20260710-142127Review Checklist