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

Lazy load dependencies #454

Merged
merged 1 commit into from
Jan 24, 2022
Merged

Lazy load dependencies #454

merged 1 commit into from
Jan 24, 2022

Conversation

rayluo
Copy link
Collaborator

@rayluo rayluo commented Jan 19, 2022

Lazy load MSAL's dependencies. This resolves #423.

Also remove an old workaround. P.S.: @xiangyan99, after this change, the Azure Identity 1.3.* and 1.4 versions are expected to throw this exception:

AttributeError: module 'msal.authority' has no attribute 'requests'

Copy link

@xiangyan99 xiangyan99 left a comment

Choose a reason for hiding this comment

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

LGTM

@jiasli
Copy link
Contributor

jiasli commented Jan 24, 2022

Actually Azure CLI already lazily imports MSAL. The issue #423 is only a (possibly not appropriate) suggestion.

I am not sure how much this change will benefit other MSAL users, who use the default requests HTTP client. But yea, lazy import requests is certainly beneficial to users how choose to use their own HTTP client.

@rayluo rayluo merged commit 636a58c into dev Jan 24, 2022
@rayluo rayluo deleted the lazy-load branch January 24, 2022 20:47
@rayluo
Copy link
Collaborator Author

rayluo commented Jan 24, 2022

Actually Azure CLI already lazily imports MSAL. The issue #423 is only a (possibly not appropriate) suggestion.

I am not sure how much this change will benefit other MSAL users, who use the default requests HTTP client. But yea, lazy import requests is certainly beneficial to users how choose to use their own HTTP client.

It turns out, lazy loading our dependencies makes MSAL easier to be used in some restricted environment. So, we will still ship this.

@jiasli
Copy link
Contributor

jiasli commented Jan 25, 2022

Could you share more info about "restricted environment"? If requests is not required, perhaps we need to tweak

@rayluo rayluo mentioned this pull request Feb 8, 2022
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.

MSAL can consider using lazy import for request, jwt
3 participants