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

[core-auth] Change GetTokenOptions to be based on OperationOptions #5899

Merged
merged 2 commits into from
Oct 29, 2019

Conversation

daviwil
Copy link
Contributor

@daviwil daviwil commented Oct 28, 2019

This PR changes GetTokenOptions to be solely based on the new OperationOptions to achieve better consistency with the API signatures in other Track 2 client libraries. For now I'm duplicating the shapes of OperationOptions and its constituent types because core-auth shouldn't take a circular dependency on core-http. We will reorganize things in the future so that this won't be necessary.

Fixes #5900

Copy link
Member

@bterlson bterlson left a comment

Choose a reason for hiding this comment

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

Happy with the changes. Timeout is still not plumbed in most credential types - we should fix this soon.

Let's run keyvault tests to ensure the credentials continue to work in practice.

@daviwil
Copy link
Contributor Author

daviwil commented Oct 28, 2019

/azp run js - keyvault - ci

@azure-pipelines
Copy link

Azure Pipelines could not run because the pipeline triggers exclude this branch/path.

@daviwil
Copy link
Contributor Author

daviwil commented Oct 28, 2019

/azp run js - keyvault-keys - tests

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@daviwil
Copy link
Contributor Author

daviwil commented Oct 28, 2019

/azp run js - keyvault-secrets - tests

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@daviwil
Copy link
Contributor Author

daviwil commented Oct 29, 2019

Approved by adpship.

@daviwil daviwil merged commit 73e041b into Azure:master Oct 29, 2019
@daviwil daviwil deleted the change-get-token-options branch October 29, 2019 00:00
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.

[core-auth] GetTokenOptions should have the same shape as OperationOptions from core-http
3 participants