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

mgmt, support HttpPipelinePosition in pipeline creation #25189

Merged
merged 2 commits into from
Nov 8, 2021
Merged

mgmt, support HttpPipelinePosition in pipeline creation #25189

merged 2 commits into from
Nov 8, 2021

Conversation

XiaofeiCao
Copy link
Contributor

fix #25060

import com.azure.core.http.policy.RequestIdPolicy;
import com.azure.core.http.policy.RetryPolicy;
import com.azure.core.http.HttpPipelinePosition;
import com.azure.core.http.policy.*;
Copy link
Member

Choose a reason for hiding this comment

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

configure your IDE to avoid *

@@ -69,15 +63,28 @@ public static HttpPipeline buildHttpPipeline(
policies.add(new AddHeadersFromContextPolicy());
policies.add(new RequestIdPolicy());
policies.add(new ReturnRequestIdHeaderPolicy(ReturnRequestIdHeaderPolicy.Option.COPY_CLIENT_REQUEST_ID));
if(additionalPolicies != null && !additionalPolicies.isEmpty()){
Copy link
Member

Choose a reason for hiding this comment

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

you probably going to fail style check.

Copy link
Member

Choose a reason for hiding this comment

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

easier to use CoreUtils.isNullOrEmpty()

@@ -0,0 +1,177 @@
package com.azure.resourcemanager.resources;
Copy link
Member

Choose a reason for hiding this comment

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

copyright

@weidongxu-microsoft
Copy link
Member

weidongxu-microsoft commented Nov 4, 2021

You may want to check with Haoling about coding styles.

see https://github.com/Azure/azure-sdk-for-java/wiki/Building for local build. (Java other than 11 or 17 might have problem on javadoc build).

@@ -69,15 +63,28 @@ public static HttpPipeline buildHttpPipeline(
policies.add(new AddHeadersFromContextPolicy());
policies.add(new RequestIdPolicy());
policies.add(new ReturnRequestIdHeaderPolicy(ReturnRequestIdHeaderPolicy.Option.COPY_CLIENT_REQUEST_ID));
if(additionalPolicies != null && !additionalPolicies.isEmpty()){
Copy link
Member

Choose a reason for hiding this comment

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

easier to use CoreUtils.isNullOrEmpty()

*/
public class HttpPipelineProviderTest {

static class MockTokenCredential implements TokenCredential {
Copy link
Member

Choose a reason for hiding this comment

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

tests is nice!

you can use mockito e.g. #24849

2. mockito use in unit test
@XiaofeiCao XiaofeiCao merged commit cc0f8d7 into Azure:main Nov 8, 2021
@XiaofeiCao XiaofeiCao deleted the handwritten_HttpPipelinePosition_support branch November 8, 2021 05:59
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.

mgmt, support HttpPipelinePosition in pipeline creation
2 participants