Enhance Sample Draft#833
Conversation
| ], | ||
| "Resource": [ | ||
| "arn:aws:iot:<b>region</b>:<b>account</b>:client/test-*" | ||
| "arn:aws:iot:<b>region</b>:<b>account</b>:client/mqtt5-sample-*" |
There was a problem hiding this comment.
Dang, we'll need to check this across all the python samples. Looking at just the x509 one we provide the wrong resource name here over there too despite the default client id being "mqtt5-sample" + a uuid.
I'm also wondering if we should remove the uuid from all client names because we no longer need them since there's no test collisions due to the samples running in CI
There was a problem hiding this comment.
I would prefer to have a uuid in client id as a "best practice". I would assume this would reduce the "duplicate id" issue/ticket get reported.
|
Let's add a table of contents jump to at the top of the x509 sample readme |
| ```Environment -> Profile (local file system) -> STS Web Identity -> IMDS (ec2) or ECS``` | ||
|
|
||
|
|
||
| ## How to build |
There was a problem hiding this comment.
Maybe this could be under "Requirements" like the other readmes with the subtopics remaining build the SDK and build the Sample. The IoT Core thing policy info should also be under the requirements section.
This readme is missing the "Introduction" header that contains the message broker and MQTT5 user guide info.
There was a problem hiding this comment.
I feel it make sense to have "Build the sample" into its own section, as it is not a "prerequisite", but directly related to the sample. "Build the SDK" might be good to put into requirement section though.
|
The parent sample folder for the mqtt5 samples should just be "mqtt" and not "mqtt5". I think that simplifies it for customers coming to the repo but I'm open to changing it to mqtt5 if there's a strong argument for it. If we do change to "mqtt5" we will need to apply the same change to the python sdk. |
| fprintf( | ||
| stdout, "Failed to Init Mqtt5Client with error code %d: %s", LastError(), ErrorDebugString(LastError())); | ||
| return -1; | ||
| stdout, "Failed to init Mqtt5Client with error code %d: %s", LastError(), ErrorDebugString(LastError())); |
There was a problem hiding this comment.
As with the other null checks above, can we remove this? If it failed, it will have failed and customer will be able to look at logs.
2 participants
Issue #, if available:
Description of changes:
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.