add interactive_browser_client_id

[xiangyan99]

No description provided.

[chlowell]

InteractiveBrowserCredential expects this argument to be set only with a valid client ID (i.e. a string). So, we should pass a value here only when interactive_browser_client_id has a string value.

[xiangyan99]

I don't see we do similar validation for other client ids. Should we keep consistent with others?

[chlowell]

InteractiveBrowserCredential(client_id=None, ...) creates a credential that doesn't work. ManagedIdentityCredential(client_id=None) creates a credential that could work, depending on the environment. Maybe both credentials should be consistent in how they handle client_id=None but they aren't today. (For what it's worth, I think InteractiveBrowserCredential is more correct here.)

[chlowell]

But the variable defaults to None, causing DefaultAzureCredential() to raise this error.

[xiangyan99]

Hah, Good catch!.

[chlowell]

How about:

browser_args = dict(kwargs, tenant_id=interactive_browser_tenant_id)
interactive_browser_client_id = kwargs.pop("interactive_browser_client_id", None)
if interactive_browser_client_id:
    browser_args["client_id"] = interactive_browser_client_id
...
if not exclude_interactive_browser_credential:
    credentials.append(InteractiveBrowserCredential(**browser_args))

[xiangyan99]

We want to pass in the client_id after validating client_id is a string, right?

Are you proposing we doing it in ctor of InteractiveBrowserCredential?

[chlowell]

We want to pass client_id if and only if the caller gave us a value for interactive_browser_client_id. My code snippet is one way to achieve that. Only a string will work for client_id but in general we don't validate types and there's no special need to do so here.

[xiangyan99]

I think we are on same page and I guess it is already addressed?

(please refresh the page, I moved the validation into

if not exclude_interactive_browser_credential:
    if interactive_browser_client_id:

)

[chlowell]

Yes, that's another way to do it. No need for the isinstance though.

[xiangyan99]

#20487

[chlowell]

This is unused now