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 SDKs to use HTTP pipeline #802

Closed
8 of 9 tasks
cataggar opened this issue Jun 14, 2022 · 6 comments
Closed
8 of 9 tasks

migrate SDKs to use HTTP pipeline #802

cataggar opened this issue Jun 14, 2022 · 6 comments
Labels

Comments

@cataggar
Copy link
Member

cataggar commented Jun 14, 2022

An HTTP pipeline general guidelines:
https://azure.github.io/azure-sdk/general_azurecore.html

A description copied from #290:


Previous to the pipeline architecture, each operation was independently controlled meaning there was no central way to control the way operations were performed.

The pipeline architecture on the other hand allows users of the SDK to create pipelines through which outgoing requests and incoming responses are processed. The pipeline are composed of "policies" which can both change the outgoing request and the incoming response. Policies are usually either on a "per-request" basis (meaning they are called only once per operation) or "per-retry" (meaning they are applied every time an operation is attempted).

Examples of policies are the TelemetryPolicy which adds various telemetry headers to the request and the retry policy which checks whether an incoming response was successful and if it detects a transient failure, retries the request.

Converting to the Architecture

For an example of converting one operation (Cosmos's get_database operation) over to the pipeline architecture, take a look at #286.

The basic steps are:

  • Create a new submodule of the operations module where the operation will live. This submodule should be named after the operation (e.g., for get_database it's called get_database).
  • Move the "request builder" (e.g., for get_database the GetDatabaseBuilder) to new operation submodule and rename it from Builder to Options (e.g., GetDatabaseBuilder => GetDatabaseOptions`). This now represents the options that can be supplied when performing the operation. Note that many options that were part of the builder are no longer valid options because there will be individual policies that take their place. For instance, previously many operations took an option to set the user agent. This is now done by a policy.
  • Change the execute method to a decorate_request method that instead of performing the request, simply mutates a request parameter with any of the options supplied.
  • Move the associated response type (e.g., for get_database the GetDatabaseResponse type from the responses submodule to the operation submodule. Previously a std::convert::TryFrom implementation was used to convert from an http::Response<Bytes> object. This should now be a plain associated async function called try_from that converts from the new Response type. See the example for how this should be done.
  • Finally, convert the appropriate method on the appropriate client (e.g., for get_database this was the DatabaseClient::get_database) to use the new options and response types with the pipeline. Again, see the example for how this should be done.

@cataggar cataggar added the Epic label Jun 14, 2022
@roeap
Copy link
Contributor

roeap commented Jun 15, 2022

@cataggar - was looking into using service principal credentials with the storage_blobs crate, and saw it is not yet supported. Looking into contributing this I think that would be a breaking change given the current design of the crate.

Rather then making an intermediate change to support identity based auth in the blobs crate, I felt it might be more sustainable to move to the pipeline architecture... That said, this is a major refractor in the crate, and I was wondering if it makes sense to adopt the generated service crate while at it. Mainly b/c I do seem to remember seeing some issue in this repo concerning the generated blob crate that might be blocking, but not sure anymore ... 😄.

@cataggar cataggar pinned this issue Jun 15, 2022
@cataggar
Copy link
Member Author

I was wondering if it makes sense to adopt the generated service crate while at it. Mainly b/c I do seem to remember seeing some issue in this repo concerning the generated blob crate that might be blocking

Let's do this epic first to migrate to the HTTP Pipeline. The epic to be on top of the generated services #803 is going to take longer. I updated that issue to explain why.

@mfrw
Copy link
Member

mfrw commented Jun 22, 2022

@cataggar I want to help out on somethings :)

@bmc-msft
Copy link
Contributor

bmc-msft commented Jun 27, 2022

Still todo:

  • iothub
  • messaging_servicebus
  • identity
  • data_tables

@roeap
Copy link
Contributor

roeap commented Aug 2, 2022

I'd be happy to give iothub a try and push it over the finish line after, since its already migrated to using operations ...

@rylev
Copy link
Contributor

rylev commented Aug 2, 2022

@roeap please feel free to!

@cataggar cataggar closed this as completed Feb 9, 2023
@cataggar cataggar unpinned this issue Feb 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants