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

add custom policy #17340

Merged
merged 8 commits into from
Mar 25, 2021
Merged

add custom policy #17340

merged 8 commits into from
Mar 25, 2021

Conversation

xiangyan99
Copy link
Member

No description provided.

@ghost ghost added the Azure.Core label Mar 15, 2021
@xiangyan99
Copy link
Member Author

#16519

@johanste
Copy link
Member

Let's align with the other languages on this. Both .NET/C# and Java uses the perCall or perRetry terminology rather than first and last. I have not checked the javascript implementation.

@xiangyan99
Copy link
Member Author

/azp run python - formrecognizer

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Comment on lines +125 to +127
if isinstance(per_call_policies, Iterable):
for policy in per_call_policies:
policies.append(policy)
Copy link
Member

Choose a reason for hiding this comment

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

No need to explicitly iterate over per_call_policies. list.extend(iterable) does the same thing.

Comment on lines +131 to +134
policies = policies + [config.redirect_policy,
config.retry_policy,
config.authentication_policy,
config.custom_hook_policy]
Copy link
Member

Choose a reason for hiding this comment

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

This causes a new list object to be created and assigned to policies. list.extend(iterable) is a better choice as it modifies policies directly.

@jiasli jiasli mentioned this pull request Apr 23, 2021
@jiasli
Copy link
Member

jiasli commented Apr 23, 2021

Fix #16519: Support adding custom policy with per_call_policies, per_call_policies

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants