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

ModuleNotFoundError: No module named 'adal' #13996

Closed
smarlowucf opened this issue Sep 23, 2020 · 8 comments
Closed

ModuleNotFoundError: No module named 'adal' #13996

smarlowucf opened this issue Sep 23, 2020 · 8 comments
Assignees
Labels
customer-reported Issues that are reported by GitHub users external to the Azure organization. Packaging question The issue doesn't require a change to the product in order to be resolved. Most issues start as that
Milestone

Comments

@smarlowucf
Copy link

smarlowucf commented Sep 23, 2020

  • Package Name: azure-common
  • Package Version: 1.1.25
  • Operating System: Ubuntu 16.04.6 LTS
  • Python Version: 3.7

Describe the bug
Attempting to use get_client_from_auth_file function from azure-common client_factory module leads to import error.

To Reproduce
Steps to reproduce the behavior:

  1. pip install azure-common
  2. Import: from azure.common.client_factory import get_client_from_auth_file
  3. Error: ModuleNotFoundError: No module named 'adal'

Expected behavior
The adal package should be required when installing azure-common as it is imported and used in the code. Thus preventing a module not found error when using get_client_from_auth_file.

Screenshots
If applicable, add screenshots to help explain your problem.

Additional context

from azure.common.client_factory import get_client_from_auth_file

../../../virtualenv/python3.7.1/lib/python3.7/site-packages/azure/common/client_factory.py:17: in <module>

    import adal

E   ModuleNotFoundError: No module named 'adal'
@ghost ghost added needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. customer-reported Issues that are reported by GitHub users external to the Azure organization. question The issue doesn't require a change to the product in order to be resolved. Most issues start as that labels Sep 23, 2020
@smarlowucf
Copy link
Author

Same issue with msrestazure package.

from azure.common.client_factory import get_client_from_auth_file

../../../virtualenv/python3.7.1/lib/python3.7/site-packages/azure/common/client_factory.py:18: in <module>

    from msrestazure.azure_active_directory import AdalAuthentication

E   ModuleNotFoundError: No module named 'msrestazure'

@smarlowucf
Copy link
Author

It looks like msrestazure has a proper requirement on adal: https://github.com/Azure/msrestazure-for-python/blob/master/setup.py#L54

It seems that azure-common should have a similar requirement on adal and msrestazure in setup.py to ensure dependency tree is installed accordingly.

@xiangyan99
Copy link
Member

@scbedd is this something you are working on?

@ghost ghost removed the needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. label Sep 24, 2020
@lmazuel
Copy link
Member

lmazuel commented Sep 24, 2020

Hi @smarlowucf
We do NOT want to add a dependency on msrestazure/adal in azure-common on purpose. This is a long story of backward compatibility and impact that this will have to suddenly install a new dependency. Most of the code in this code is catching ImportError to explain that auth features requires additional installation of `msrestazure manually.

try:
from msrestazure.azure_active_directory import (
InteractiveCredentials,
ServicePrincipalCredentials,
UserPassCredentials
)
except ImportError:
raise ImportError("You need to install 'msrestazure' to use this feature")

We did have a extra node to install like pip install azure-common[msrestazure], but given how pip was handling this badly when it was used as a dependency, and the number of people complaining that it was forgetting often to install msrestzure, we removed it.

We could though make the same ImportError catcher in client_factory, to make the error message better, but we won't add the dependency. Given we are slowly deprecating azure-common anyway, and that this won't change the fact that you need to manually install msrestazure too, I don't think that's a priority.

Thanks!

@lmazuel lmazuel assigned lmazuel and unassigned scbedd Sep 24, 2020
@lmazuel lmazuel added this to the Backlog milestone Sep 24, 2020
@smarlowucf
Copy link
Author

Hi, @lmazuel thanks for the explanation. IMHO it's not nice to force downstream packages to have dependencies on packages that really are used upstream. This can become very troublesome to properly manage. The downstream packages now have to add all this in their own requirements.txt:

azure-common
adal
msrestazure

Which is quite confusing and inaccurate because adal and msrestazure are not used in those packages directly.

@smarlowucf
Copy link
Author

smarlowucf commented Sep 24, 2020

This issue has only come up recently which means a backwards incompatible change was made somewhere in the Azure packages. In my case I have requirements on:

azure-common
azure-mgmt-compute
azure-mgmt-network
azure-mgmt-resource

Thus while some backwards incompatible changes are being avoided (azure-common) it seems that is not the case in all packages?

@lmazuel
Copy link
Member

lmazuel commented Nov 23, 2020

Re-testing this with 1.1.26 gives the following error:

>>> from azure.common.client_factory import get_client_from_auth_file
Traceback (most recent call last):
  File "/tmp/testmod/lib/python3.8/site-packages/azure/common/credentials.py", line 107, in <module>
    from msrest.authentication import (
ModuleNotFoundError: No module named 'msrest'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/tmp/testmod/lib/python3.8/site-packages/azure/common/client_factory.py", line 17, in <module>
    from .credentials import get_azure_cli_credentials
  File "/tmp/testmod/lib/python3.8/site-packages/azure/common/credentials.py", line 113, in <module>
    raise ImportError("You need to install 'msrest' to use this feature")
ImportError: You need to install 'msrest' to use this feature

Which is the best we can do today considering our need for backward compatibility, and the fact that more and more packages are actually incompatible with "get_client_from_auth_file" nowadays (see #15075). Also, if you have a package compatible, the package in question will install "msrest" anyway.

Closing this issue giving the preceding reasoning.

@lmazuel lmazuel closed this as completed Nov 23, 2020
@smarlowucf
Copy link
Author

Hi @lmazuel can you please point me to documentation describing how to use the new authentication route? I'm very confused and have no luck finding any examples or docs. It would be helpful to see an entire workflow including authentication of a client. For example the code that worked before:

from azure.common.client_factory import get_client_from_auth_file
from azure.mgmt.compute import ComputeManagementClient


compute_client = get_client_from_auth_file(
    ComputeManagementClient,
    auth_path={auth_file...}
)
async_create_image = compute_client.images.create_or_update(...)

If get_client_from_auth_file is legacy then what is replacing this and what's the new workflow?

@github-actions github-actions bot locked and limited conversation to collaborators Apr 12, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
customer-reported Issues that are reported by GitHub users external to the Azure organization. Packaging question The issue doesn't require a change to the product in order to be resolved. Most issues start as that
Projects
None yet
Development

No branches or pull requests

4 participants