Autorest: [Python] Credentials parameter should not be mandatory

[lmazuel]

In the Fabric Swagger, generated with "Python" (and not Azure.Python), credentials are optional (service endpoint can be configured on http instead of https). However, it seems Autorest always generate this parameter and check its existence. This means that for an unsecured Fabric endpoint, I have to create the client this way:

client = azure.fabric.ServiceFabricClient(
    api_version='2.0',
    base_url='http://mycluster.westus.cloudapp.azure.com:19080',
    credentials = 'fake'
)

The client works well and the operations are ok, so it's a proof that credentials is not used at all.

I think it's a python issue, but the Swagger might have a mistake too (not sure what to check)

FYI @annatisch

[annatisch]

@lmazuel
My understanding is that credentials are handled by the generator externally from the spec. The presence of the credentials object is fully dictated by the AddCredentials=true/false used in the autorest.exe command.
Currently we handle this as:
AddCredentials=false -> Always an http endpoint
AddCredentials=true -> Always an https endpoint

Clearly this doesn't work for the service fabric API. Does that mean that the other generators interpret AddCredentials=true to mean that the credentials should always be optional? As I can't see anywhere else where this optional/required distinction could be made.....

[lmazuel]

@annatisch Ok, I understand. For service fabric both are ok, it's even a question the Portal asks when you create the cluster: would you like http or https? And for the record the customer that asks me for Fabric was using too http.

@fearthecowboy What do you think? AddCredentials==true, means :
1- we add an optional Credentials parameter
2- we add a required Credentials parameter

[annatisch]

@fearthecowboy - any thoughts? Do you know if this client works in C# or other lanugages? If so then I guess we can assume that AddCredentials means the credentials should be optional in all cases?

[lmazuel]

In Java it's optional.

Comparing what in the NodeJS template with the generated code, it seems it's required.

[lmazuel]

@amarzavery based on what I see in the Node repo, you don't use "AddCredentials" parameter while calling Autorest for NodeJS. Why is that?

[fearthecowboy]

Sorry, rushing to get so many things done this week :D

I think even if we could make a solid statement one way or another, we'd only be able to really enforce that for 'Azure ' generators, except they should always have some kind of Auth, right?

Grsh.

I think we should default to a reasonable default for now, and I'll make sure we add in the ability in Configuration to override the behavior.

And I still can't make up my mind either way.

My gut tells me that AddCredentials= true means add a required parameter.

[fearthecowboy]

Most generators Enforce AddCredentials=true for 'Azure' generators

[lmazuel]

Ho, so Azure generators should not propose at all AddCredentials and force it? That's interesting.

I guess we can close this issue saying that it's required. I just realize that ServiceFabric is not using "credentials" in the NodeJS SDK (used for the XPlatCLI) and seems to not be a problem, I guess this is authenticating one way on another when on https (but not the "Autorest way").

Thanks!

[amarzavery]

As far as I understand, for Azure version of the code generator credentials is a mandatory parameter. We added a Boolean swtich -AddCredentials that can be used for the generic version and that will make credentials a required parameter while instantiating a client that was generated from the generic version. Why is there so much confusion about this after almost 2 years of this project?

[lmazuel]

I assume to be confused, not in the Autorest team and not here for two years :)
So in summary:

• Generic generator:
  AddCredentials=false (default), no Credentials parameter
  AddCredentials=true, _required_ Credentials parameter
• Azure generator:
  AddCredentials is true all the time.

Autorest does not help to understand that:

Parameters:
  -AddCredentials: If true, the generated client includes a ServiceClientCredentials property and constructor parameter. Authentication behaviors are implemented by extending the ServiceClientCredentials type.

Nothing says about a change in behavior depending of the Generator used (ìf true means default is false which is not the case for Azure generators it seems), nor the required/optional status of the added property.

[amarzavery]

Ahh I see.. You are right.. It should be documented that -AddCredentials is always true for Azure Generator.

[lmazuel]

@fearthecowboy guess this is another thing like https://github.com/Azure/autorest/issues/1647 that you want to enforce in a "Core for Azure" part :)

[olydis]

This is handled by https://github.com/Azure/autorest/blob/86ad96234e55ccb8f13e033fdfc5e377e2831d99/src/core/AutoRest.Extensions.Azure/AzureExtensions.cs#L59
so generators at least have the option to handle this consistently.

If this is still an issue in python, please reopen the issue.