This repository has been archived by the owner on Apr 26, 2024. It is now read-only.
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.
[WIP] device dehydration #7955
[WIP] device dehydration #7955
Changes from 6 commits
52ddb79
e60a99d
0f9e402
b59bc66
96d9fc3
460ebc5
c7c8f28
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.
The documentation about what this callback might be (see the
check_auth
info in there) is less than ideal. It comes from password auth modules and might beNone
or aCallable
that is "called with the result from the/login
call (includingaccess_token
,device_id
, etc.)"I do not know of any auth providers which use this frankly, but I think calling it like below is the reasonable thing to do. Is there a particular reason you think it should not be called?
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.
We talked a bit more about this and the reason we can't just call it here is that we don't yet have the access token (the client hasn't acknowledged that they've been able to use the dehydrated device).
It seems like we would ideally want to call this at the end of the call to
/restore_device
, but we no longer have the callback at that point. Will need to think about what the best option is here.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.
Not sure if this is supposed to show up only in
unstable
or alsor0
(the current code would do both).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.
We probably want to use
assert_params_in_dict
fordehydration_token
since it is required -- this will raise a sane error message. (This is true for any parameters that are required in the added endpoints.)