Move delete_user and list_users to pipeline architecture #346
Conversation
| ctx: Context, | ||
| options: ListUsersOptions, | ||
| ) -> impl Stream<Item = Result<ListUsersResponse, crate::Error>> + '_ { | ||
| macro_rules! r#try { |
There was a problem hiding this comment.
We'll want to move most of this logic to a shared place once this and #340 lands.
| @@ -135,21 +158,6 @@ impl UserClient { | |||
| PermissionClient::new(self, permission_name) | |||
| } | |||
|
|
|||
| pub(crate) fn prepare_request_with_user_name( | |||
There was a problem hiding this comment.
I think we actually want to keep this and just make sure that replace_user is also using it.
There was a problem hiding this comment.
Good catch, I pushed a change that adds it back and uses it in Get, Replace, and Delete
|
@JoshGendein can we close this PR now that #349 has been merged? |
I was going to wait for #340 to be merged in. Ryan mentioned we could move some of the code to a shared place. |
|
ohh, my bad! - When I suggested we "close this PR", I misread which APIs this was about and thought it was a duplicate 😅 I think this PR should probably be good to merge now! We're tracking "shared abstractions for streaming responses" in #341, so we can work on creating that once both this PR and #340 have been merged. It's usually easier to find a shared abstraction once there's multiple things that need it anyway (: tldr: once we resolve the merge conflict, let's merge it? |
We should probably merge #339 first because without it we cannot test if these changes still work against Azure. |
@yoshuawuyts @MindFlavor Ok, I fixed the merge conflict so this is ready to be completed. Should this still wait for the auth token PR to be merged? |
|
@JoshGendein Sorry for not merging this sooner. Can you rebase again, and then we can merge. |
Shared UserResponse to replace CreateUserResponse, GetUserResponse, and ReplaceUserResponse
delete_user to pipeline architecture
list_users to pipeline architecture using stream implementation
Continuing #290