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

MultiAddressHttpClientBuilder deprecation and de-deprecation refactoring #1722

Merged
merged 2 commits into from
Aug 11, 2021

Conversation

chemicL
Copy link
Contributor

@chemicL chemicL commented Aug 6, 2021

Motivation:

The MultiAddressHttpClientBuilder deprecated methods which should stay
and allow configuring the ExecutionContext (ioExecutor,
executionStrategy, bufferAllocator). Furthermore, methods
influencing the client filters have not been deprecated, but should, as
the SingleAddressInitializer allows configuring the filters in a
satisfactory fashion.
This PR is a prequel to #1721 which introduces an intended cleanup of the MultiAddressHttpClientBuilder API.

Modifications:

  • Removed @Deprecated annotation from ioExecutor, executionStrategy,
    and bufferAllocator methods,
  • Deprecated MultiAddressHttpClientFilterFactory,
  • Deprecated appendClientFilter methods accepting
    MultiAddressHttpClientFilterFactory,
  • Deprecated StreamingHttpClientFilterFactory#asMultiAddressClientFilter.

Result:

The current API reflects the future direction for
MultiAddressHttpClientBuilder and how it should be used.

Motivation:

The MultiAddressHttpClientBuilder deprecated methods which should stay
and allow configuring the `ExecutionContext` (`ioExecutor,
`executionStrategy`, `bufferAllocator`). Furthermore, methods
influencing the client filters have not been deprecated, but should, as
the `SingleAddressInitializer` allows configuring the filters in a
satisfactory fashion.

Modifications:

- List the changes
- Removed `@Deprecated` annotation from `ioExecutor`, `executionStrategy`,
  and `bufferAllocator` methods,
- Deprecated `MultiAddressHttpClientFilterFactory`,
- Deprecated `appendClientFilter` methods accepting
  `MultiAddressHttpClientFilterFactory`.

Result:

The current API reflects the future direction for
`MultiAddressHttpClientBuilder` and how it should be used.
*/
@Deprecated
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Following @Scottmitch comment from #1721 (comment) I also added deprecation for this method. Please re-review @tkountis @Scottmitch.

@tkountis
Copy link
Contributor

tkountis commented Aug 9, 2021

lgtm

@chemicL chemicL merged commit be18dbe into apple:main Aug 11, 2021
chemicL pushed a commit that referenced this pull request Aug 11, 2021
…oring (#1722)

MultiAddressHttpClientBuilder deprecation and de-deprecation refactoring

Motivation:

The `MultiAddressHttpClientBuilder` deprecated methods which should stay
and allow configuring the `ExecutionContext` (`ioExecutor,
`executionStrategy`, `bufferAllocator`). Furthermore, methods
influencing the client filters have not been deprecated, but should, as
the `SingleAddressInitializer` allows configuring the filters in a
satisfactory fashion.

Modifications:

- Removed `@Deprecated` annotation from `ioExecutor`, `executionStrategy`,
  and `bufferAllocator` methods,
- Deprecated `MultiAddressHttpClientFilterFactory`,
- Deprecated `appendClientFilter` methods accepting
  `MultiAddressHttpClientFilterFactory`,
- Deprecated `StreamingHttpClientFilterFactory#asMultiAddressClientFilter`.

Result:

The current API reflects the future direction for
`MultiAddressHttpClientBuilder` and how it should be used.
Copy link
Member

@idelpivnitskiy idelpivnitskiy left a comment

Choose a reason for hiding this comment

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

Late LGTM! Thanks!

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.

4 participants