Skip to content

Read the ClientRequestId off initial request#11304

Merged
pakrym merged 4 commits intoAzure:masterfrom
pakrym:pakrym/read-custom-id
Apr 14, 2020
Merged

Read the ClientRequestId off initial request#11304
pakrym merged 4 commits intoAzure:masterfrom
pakrym:pakrym/read-custom-id

Conversation

@pakrym
Copy link
Copy Markdown
Contributor

@pakrym pakrym commented Apr 14, 2020

I had to move the policy to be the first one in the pipeline so everyone can read the correct value of the Request.ClientRequestId. I couldn't come up with a scenario where it's breaking.

Fixes: #11198

@pakrym pakrym requested a review from tg-msft April 14, 2020 16:41
}

[Test]
public async Task CustomClientRequestIdAvailableInPerCallPolicies()
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Tests policy ordering.

Copy link
Copy Markdown
Member

@tg-msft tg-msft 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!

@KrzysztofCwalina
Copy link
Copy Markdown
Member

I couldn't come up with a scenario where it's breaking.

Good that we did not expose index-based API add new policies to client options :-)

@pakrym
Copy link
Copy Markdown
Contributor Author

pakrym commented Apr 14, 2020

This change is breaking, if perCallPolicies were setting ClientRequestId value with intent of it turning into a header value it won't work anymore.

I'm going to split this policy in two.

}

[Test]
public async Task CustomClientRequestIdSetInPerCallPolicyAppliedAsAHeader()
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Sorry for causing this problem - but thank you for making the scenario work.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It's a good test I with I had before.

@pakrym
Copy link
Copy Markdown
Contributor Author

pakrym commented Apr 14, 2020

/azp run net - core - ci

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@pakrym pakrym merged commit 84c629c into Azure:master Apr 14, 2020
@pakrym pakrym deleted the pakrym/read-custom-id branch April 14, 2020 23:35
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.

ClientRequestId policy should read value of message if it's availible

3 participants