Skip to content
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

Migrate iothub to pipelines #972

Merged
merged 3 commits into from
Aug 4, 2022
Merged

Migrate iothub to pipelines #972

merged 3 commits into from
Aug 4, 2022

Conversation

roeap
Copy link
Contributor

@roeap roeap commented Aug 2, 2022

part of #802

Migrating the iothub sdk to pipelines architecture.

I tried to retain the API surface. There are some methods that return generic CollectedResponse and implement the TryInto for a specific return type. In these cases I implemented the "async try_from" that is generally used on the type alias, as to not have conflicts / ambiguities. An example being get_identity used for module and device identity methods.

I also added an authorization policy along with the option to use identities for authorization. I never really worked with iothub, but looking at the docs it seems that is a valid auth method. Currently there is no way to specify client options, albeit them being used for constructing the pipeline. This was deliberate, as I understand a new builder pattern for the clients is being worked on and there were no options before as well.

Lastly I reused the timeout policy from storage and moved it to core.

@rylev @ctaggart

Copy link
Contributor

@rylev rylev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! I agree that there are still some open questions, but this brings us much closer to where we want to be. I had some small nits - feel free to address them here or merge, and we can do a follow up.

sdk/iot_hub/src/service/mod.rs Outdated Show resolved Hide resolved
@roeap
Copy link
Contributor Author

roeap commented Aug 3, 2022

Thanks @rylev, adressed the PR comments, you'll have to do the merging :)

@rylev rylev merged commit eeb3176 into Azure:main Aug 4, 2022
@rylev rylev mentioned this pull request Aug 4, 2022
9 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants