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

SyncStack - Investigate usage of BinaryData as HttpRequest body carrier. #26784

Closed
kasobol-msft opened this issue Jan 28, 2022 · 3 comments
Closed
Assignees
Labels
Azure.Core azure-core Client This issue points to a problem in the data-plane of the library.

Comments

@kasobol-msft
Copy link
Contributor

kasobol-msft commented Jan 28, 2022

The HttpRequest uses Flux<ByteBuffer> to carry body from upper layers to the HttpClient. Consider changing that carrier to something that defers assumption of what's really backing the payload (whether it's stream, buffer, byte[] or flux) until absolutely necessary.
For example today binary payloads are eagerly converted to flux in convenience layers - and then often converted to something else at http client layer (apart from reactor-netty that speaks flux). If somebody is providing InputStream as upload data source then the decision how to use that should be up to http client how best consume that payload.

Known challenges:

  • RestProxy - how do we validate length for different BinaryDataContent types?
  • RetryPolicy - This is tightly coupled to Flux operators. How do we retry?
  • Should we generalize beyond BinaryData ?
  • How do we leverage each http client's consumption options to minimize transformations?
  • How do we leverage seekability of files or in memory buffers to remove body buffering for some scenarios?

The usage of HttpRequest.body is widespread. Those more difficult usage patterns (like RetryPolicy) might be deferred (i.e. conversion to flux) and a work item to cover that created. The priority should be to cover sync/async RestProxy logic first.

@kasobol-msft kasobol-msft added Client This issue points to a problem in the data-plane of the library. Azure.Core azure-core labels Jan 28, 2022
@kasobol-msft kasobol-msft changed the title Investigate usage of BinaryData as HttpRequest body carrier. SyncStack - Investigate usage of BinaryData as HttpRequest body carrier. Feb 11, 2022
@kasobol-msft kasobol-msft self-assigned this May 11, 2022
@kasobol-msft
Copy link
Contributor Author

#28737

@kasobol-msft kasobol-msft moved this to In Progress in Java SDKs Azure Core May 11, 2022
@kasobol-msft
Copy link
Contributor Author

#28829

@kasobol-msft
Copy link
Contributor Author

kasobol-msft commented May 31, 2022

#29084

#28889

Repository owner moved this from In Progress to Done in Java SDKs Azure Core May 31, 2022
@github-actions github-actions bot locked and limited conversation to collaborators Apr 11, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Azure.Core azure-core Client This issue points to a problem in the data-plane of the library.
Projects
Status: Done
Development

No branches or pull requests

1 participant