Python: IoT Basics Backfill #7700
Conversation
github-actions
bot
added
the
Python
tejasgn1
added
the
Basics
rlhagerm
left a comment
rlhagerm
left a comment
There was a problem hiding this comment.
Just some comments and questions.
| <!--custom.prerequisites.start--> | ||
| <!--custom.prerequisites.end--> | ||
|
|
||
| ### Get started |
There was a problem hiding this comment.
The folder is missing the requirements.txt file.
|
|
||
| ### Get started | ||
|
|
||
| - [Hello AWS IoT](iot_hello.py#L14) (`listThings`) |
There was a problem hiding this comment.
The README and the Spec indicate that the Hello example should use ListThings, but it seems to only print out the endpoint.
| print("\n" + "-" * 88) | ||
| print("9. List topic rules") | ||
| print("-" * 88) | ||
| rules = self.iot_wrapper.list_topic_rules() |
There was a problem hiding this comment.
This doesn't actually print the rules, just the count.
|
|
||
| def _wait(self, msg): | ||
| """Wait for user to press Enter.""" | ||
| input(f"\n{msg} Press Enter to continue...") |
There was a problem hiding this comment.
Isn't there a method for this in demo tools?
|
|
||
| def __init__(self, iot_client, iot_data_client=None): | ||
| """ | ||
| :param iot_client: A Boto3 AWS IoT client. |
There was a problem hiding this comment.
Usually we have a from_client method here to assist with the setup. See the other scenarios for an example.
| :return: The name and ARN of the created thing. | ||
| """ | ||
| try: | ||
| response = self.iot_client.create_thing(thingName=thing_name) |
There was a problem hiding this comment.
Each action has a specified exception type to handle, make sure they are all handled. For example, create_thing should handle ResourceAlreadyExistsException according to the specification.
| paginator = self.iot_client.get_paginator("list_things") | ||
| for page in paginator.paginate(): | ||
| things.extend(page["things"]) | ||
| logger.info("Retrieved %s things.", len(things)) |
There was a problem hiding this comment.
The method says it lists the things, but it seems to just print a count. Also, no specific exception handling, same for the rest of the methods.
| paginator = self.iot_client.get_paginator("list_certificates") | ||
| for page in paginator.paginate(): | ||
| certificates.extend(page["certificates"]) | ||
| logger.info("Retrieved %s certificates.", len(certificates)) |
There was a problem hiding this comment.
Same comment, are we listing or just counting the results?
| ] | ||
|
|
||
| with patch("time.sleep"): | ||
| with patch.object(scenario, "_deploy_stack", return_value=(f"arn:aws:sns:{region}:{account_id}:test", f"arn:aws:iam::{account_id}:role/test")): |
There was a problem hiding this comment.
I'm not sure why it needs a patch for these? It's also running so quickly that I doubt it's running an actual integration test. See some of the example integration tests in other services for some examples of how it should look.
| with patch.object(iot_wrapper, "create_topic_rule"): | ||
| scenario.run_scenario("test_thing", "test_rule") | ||
|
|
||
| assert True |
There was a problem hiding this comment.
Why does it assert True? At minimum, it should assert that the final text is in the captured.out, indicating the full scenario ran and made it to the end without errors.
2 participants
This pull request adds the Python IoT basics scenario.
CDD build: Link
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.