setting use_async=True for get_async_default_azure_credential #35432
Conversation
…t_azure_credential
boring-cyborg
bot
added
area:providers
provider:microsoft-azure
| assert "workload_identity_tenant_id" in widgets | ||
|
|
||
|
|
||
| @mock.patch(f"{MODULE}.DefaultAzureCredential") |
There was a problem hiding this comment.
@Lee-W, the changes look good. Do you think it is a good idea to elaborate the unit tests a bit by adding mock objects and running tests to compare expected vs. actual for the method calls?
There was a problem hiding this comment.
I'm not sure. I think the main point here is verifying we're calling the right object
There was a problem hiding this comment.
Yeah - for me it's enough to check they are called. Too detailed tests are often problematic - especially in case of some auth methods that tend to change when client libraries change. We often have to fix such tests when new versions of libraries are released just to add a new parameter expected etc. So I'd say just "called" is good enough for me - especially that we have detailed test for the credentials class itself
…t_azure_credential (apache#35432)
ephraimbuddy
added
the
changelog:skip
4 participants
This was messed up when working on fixing the typing issue on https://github.com/apache/airflow/pull/35394/files#diff-107c8c5868dad1579df1506bfae6a93f38cb51c9120ab11ce26a695e3c8a8464R91. Just fixed it and add a few test cases
^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named
{pr_number}.significant.rstor{issue_number}.significant.rst, in newsfragments.