async DefaultAzureCredential incompatible with AsyncTokenCredential per pyright

[kristapratico]

Pyright reports a type error when the async DefaultAzureCredential is used anywhere where an AsyncTokenCredential is expected:

  C:\Users\krpratic\azure-sdk-for-python\sdk\cognitivelanguage\azure-ai-language-conversations\samples\async\sample_authentication_async.py:57:66 - error: Argument of type "DefaultAzureCredential" cannot be assigned to parameter "credential" of type "AzureKeyCredential | AsyncTokenCredential" in function "__init__"
    Type "DefaultAzureCredential" cannot be assigned to type "AzureKeyCredential | AsyncTokenCredential"
      "DefaultAzureCredential" is incompatible with "AzureKeyCredential"
      "DefaultAzureCredential" is incompatible with protocol "AsyncTokenCredential"
        "__aenter__" is an incompatible type
          Type "() -> Coroutine[Any, Any, DefaultAzureCredential]" cannot be assigned to type "() -> Coroutine[Any, Any, None]"
            Function return type "Coroutine[Any, Any, DefaultAzureCredential]" is incompatible with type "Coroutine[Any, Any, None]"
        "__aexit__" is an incompatible type
          Type "(*args: Unknown) -> Coroutine[Any, Any, None]" cannot be assigned to type "(exc_type: Unknown, exc_value: Unknown, traceback: Unknown) -> Coroutine[Any, Any, None]"

This is due to the difference in how the Protocol is typed vs. how it is typed in identity:

azure-sdk-for-python/sdk/core/azure-core/azure/core/credentials_async.py

Lines 28 to 35 in ac0672f

	async def close(self) -> None:
	pass
	async def __aenter__(self):
	pass
	async def __aexit__(self, exc_type, exc_value, traceback) -> None:
	pass

azure-sdk-for-python/sdk/identity/azure-identity/azure/identity/aio/_internal/__init__.py

Lines 11 to 20 in ac0672f

	class AsyncContextManager(abc.ABC):
	@abc.abstractmethod
	async def close(self):
	pass
	async def __aenter__(self):
	return self
	async def __aexit__(self, *args):
	await self.close()

Note that this doesn't occur with the sync credential since __enter__ and __exit__ are not described by TokenCredential.

[kristapratico]

I think it would be less disruptive to update AsyncTokenCredential (although I'm not sure if this would be considered breaking):

 async def close(self) -> None: 
     pass 
  
 async def __aenter__(self) -> typing.Self: 
     pass 
  
 async def __aexit__(self, *args: Any) -> None: 
     pass 

[xiangyan99]

@mccoyp is this something you are working on? (I want to avoid dup the work)

[mccoyp]

@xiangyan99 Looks like pyright is more strict than mypy, and in a similar boat as PyCharm's type checker. #25788 should resolve this.

When I last spoke with Laurent, he feels that we should update our type signatures to correctly implement the TokenCredential protocols (even when some of the kwargs can't be used, like tenant_id with ManagedIdentityCredential). We would just need to clearly document when these kwargs aren't being used.

[mccoyp]

I think it would be less disruptive to update AsyncTokenCredential (although I'm not sure if this would be considered breaking):

 async def close(self) -> None: 
     pass 
  
 async def __aenter__(self) -> typing.Self: 
     pass 
  
 async def __aexit__(self, *args: Any) -> None: 
     pass 

After taking a look at reference docs, I do think updating the protocol definition would be a better option since examples use *exc instead of exception type, value, and traceback positional parameters:

async def __aexit__(self, *exc):
    print('Finishing')
    return False

That said, I do think it would be a breaking change to put these named parameters into *args. You would no longer be able to pass named arguments in method calls without getting an "unexpected keyword argument" error. The question is whether this is a real scenario that we need to worry about; someone would have had to exactly implement the protocol and call the dunder method directly in order to see this.

[mccoyp]

Resolved by #31494.

[kristapratico]

Re-opening, needed to rollback the code that fixed this error. Details in #31585

[pvaneck]

With the changes in #31585, seems like the fix for this is to update the __aexit__ implementations in azure-identity with the updated signature?

Like here.

    async def __aexit__(
        self,
        exc_type: Optional[Type[BaseException]] = None,
        exc_value: Optional[BaseException] = None,
        traceback: Optional[TracebackType] = None,
    ) -> None:
         await self.close()

[xiangyan99]

Seems like https://github.com/Azure/azure-sdk-for-python/pull/31578/files can fix it. :)