Azure servicebus transport has hidden dependency on azure-identity package

[Korijn]

The azure-servicebus transport depends on azure-identity package, even though it is not listed:

https://github.com/celery/kombu/blob/d620132ecee40fc021f7a78750dfe01331e8a8c0/requirements/extras/azureservicebus.txt#L1

It looks like the code intends for it to be optional:

https://github.com/celery/kombu/blob/d620132ecee40fc021f7a78750dfe01331e8a8c0/kombu/transport/azureservicebus.py#L72-L77

But of course it crashes here when (effectively) calling isinstance(..., None)

https://github.com/celery/kombu/blob/d620132ecee40fc021f7a78750dfe01331e8a8c0/kombu/transport/azureservicebus.py#L140-L146

File "/app/.venv/lib/python3.9/site-packages/kombu/transport/azureservicebus.py", line 144, in _try_parse_connection_string
    if (isinstance(self._credential, DefaultAzureCredential) or
TypeError: isinstance() arg 2 must be a type or tuple of types

This problem was introduced in #1641

[Nusnus]

This problem was introduced in #1641

@jasonwbarnett hey friend - can you check this out please?

[jesusgarmanz]

Hi! Seems like this is the same issue I found on #1947

[Nusnus]

@Korijn @jesusgarmanz any of you want to offer a PR to fix this issue? 🙂

[jasonwbarnett]

@Nusnus I got this.

[jasonwbarnett]

This break was introduced in #1801, not #1641

You know what. The original intent was to make this optional. In other words, don't force azure-identity down people's throats. They should only install it, optionally, if they require the use of either of the new features introduced in #1641

That said, I could update the code to throw an exception detailing what steps are necessary if they wish to use it and update the docs. I don't think adding a forced requirement makes sense here.

Thoughts?

[Korijn]

Yeah, that's what I said:

It looks like the code intends for it to be optional:

But it's implemented in a way that it isn't actually optional.

[jasonwbarnett]

@Korijn I would like input from @marnikow. He introduced the isinstance() check which introduced this bug. I don't know what the isinstance() check does here. I no longer have the job where I was using this code in production for over a year (code without @marnikow fixes). I need input from him so I can get more context.

[jasonwbarnett]

Actually, I just hacked this into place. This should work now.

[jasonwbarnett]

@Korijn can you update the issue description so that it accurately references #1801 as the PR which introduced the bug you've reported?

[Korijn]

@Korijn can you update the issue description so that it accurately references #1801 as the PR which introduced the bug you've reported?

Done!