Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Endpoint redirection documentation #447
Endpoint redirection documentation #447
Changes from 1 commit
1eda294
f952a9f
0dff9f8
9fb0d38
76b61ba
85314a0
f9166a7
968707d
5bc4a31
6cd768f
4e8be1f
60109f6
5475e3e
ae5d8ac
9b175ef
1e8cfa9
8ff7d97
b88b56a
258a3e9
93e3664
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pedantically there should be a check at the start that verifies that
state.blobset.blobs._iot_endpoint_config == null
(as the config block is empty so there should be no corresponding state).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should add an additional config/state sequence at the end that starts with
config.blobset.blobs._iot_endpoint_config = null
and then we verify that thestate.blobset.blobs._iot_endpoint_config == null
The fundamental reason is that we want to verify that the state block only shows up when expected.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there should be a check in here that the system.last_update time of the state message matches the config timestamp from the new endpoint (so might need to indicate that too). Goal is to check that they don't just connect and then use the wrong reply.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How long does
phase: final
stay in the state message for? Atleast 1? Noting also that the new endpoint probably won't have ablobset.blob._iot_endpoint_config
blobThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added last_update and timestamps for config and state messages
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what about a time limit here? I'd be worried that somebody implements a "try for 3h before reporting failure" scenario... so we should just have a fixed time-bound (which will be needed for the test anyway)... 30s? We can always increase it later, and devices can always report failure faster.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added note for 30 seconds
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
somewhere we need to introduce a category(s) for this too...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we differentiate for different types of failure? Or is it a single
endpoint.reconfig.failed
or similar?A pretty common error I've encountered before is an endpoint "unreachable" because of firewall or network restrictions. Would that be its own category, or is that level of detail left to the message?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Category added