Autorest: `Client` suffix for Swagger title and Autorest

[lmazuel]

Hi @amarzavery @fearthecowboy @markcowl

I see that regularly now, new Swagger files remove the Client suffix from the title in Swagger (recently new Storage 2015-05-01, changed from StorageManagementClient to StorageManagement). I remember having a discussion with @amarzavery where he told me that this is a good behavor, since the Swagger file can describe also the server and is not client only. BUT Autorest might add Client to the main class name if not present.

I wanted to check with you what you think about this. Since this is the only breaking change for the new Storage file in Python, I'd like to have your feeling before release it (if Autorest adds a Client suffix in the main class name if not present, this is not breaking). If generated clients should not have Client and have to follow strictly the title, (and the client should be named StorageManagement for the storage example), that's ok too (I saw the C# PR integrates the change, so it might be on purpose). Just tell me :)

Thanks!

FYI @annatisch

[olydis]

Sounds like this was added with #1665 :-)

[olydis]

Although, I see the PR only affects C#/Azure code generation.
@fearthecowboy Should this be a fix for all azure generators in the long run?

[lmazuel]

Thanks @olydis !

@fearthecowboy This is blocking for Storage Mgmt release in Python. Could you confirm that each language should add the Client suffix?

@amarzavery for Node, @veronicagg for Ruby, @jianghaolu for Java

[amarzavery]

The fix is made in the Swagger modeler. No op for the generator. If you try to generate a py client for the storage spec it should have the Client suffix in the client name.

Get Outlook for Androidhttps://aka.ms/ghei36

---

From: Laurent Mazuel [e-mail]
Sent: Monday, December 12, 2016 4:41:04 PM
To: Azure/autorest
Cc: Amar Zavery; Mention
Subject: Re: [Azure/autorest] Client suffix for Swagger title and Autorest (#1647)

Thanks @olydishttps://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Folydis&data=02%7C01%7Camzavery%40microsoft.com%7C7167bdfe197e478c445708d422f0b87f%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636171864658719235&sdata=tQwxhu6FnRqcVQyL4iN%2F1MFXw4NoVDynYg6q6FhsoZU%3D&reserved=0 !

@fearthecowboyhttps://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Ffearthecowboy&data=02%7C01%7Camzavery%40microsoft.com%7C7167bdfe197e478c445708d422f0b87f%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636171864658719235&sdata=09NLlre60%2FX0J1SE4drvWcWadWn6qZDv3c19O4lOvoE%3D&reserved=0 This is blocking for Storage Mgmt release in Python. Could you confirm that each language should add the Client suffix?

@amarzaveryhttps://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Famarzavery&data=02%7C01%7Camzavery%40microsoft.com%7C7167bdfe197e478c445708d422f0b87f%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636171864658719235&sdata=nP9otagbfEWeGyTlpW9MVA8uSL2HLd%2Fq586J5ow0uOQ%3D&reserved=0 for Node, @veronicagghttps://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fveronicagg&data=02%7C01%7Camzavery%40microsoft.com%7C7167bdfe197e478c445708d422f0b87f%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636171864658719235&sdata=dWGW9a0lKJnmlscH8zm%2Bi9qg5ATFAgjQjrrcNLq3%2BwE%3D&reserved=0 for Ruby, @jianghaoluhttps://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fjianghaolu&data=02%7C01%7Camzavery%40microsoft.com%7C7167bdfe197e478c445708d422f0b87f%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636171864658719235&sdata=rORF6y9PNDdxr58fNgCRuCaBtjIFwYOwtNeoD9VmGMo%3D&reserved=0 for Java

-
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHubhttps://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2FAzure%2Fautorest%2Fissues%2F1647%23issuecomment-266599967&data=02%7C01%7Camzavery%40microsoft.com%7C7167bdfe197e478c445708d422f0b87f%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636171864658719235&sdata=HxGlsvae71tEBD9CKRBF%2BKTWRToVCyKfSvz%2FRN8rr7U%3D&reserved=0, or mute the threadhttps://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FAET9B100oC9L9-hOe143QNH8qjSERFfvks5rHemggaJpZM4LAiX0&data=02%7C01%7Camzavery%40microsoft.com%7C7167bdfe197e478c445708d422f0b87f%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636171864658729239&sdata=IZPr2eN0BFUBhfH6fZ3THlLfiLq8N1zH5r6G5yoJhrw%3D&reserved=0.

[fearthecowboy]

It should affect each langauge (in base CodeModel class, the Name member was changed to Fixable<string>, and the implementation tacks on Client if it's not present.

@lmazuel if you're sync'd up to master and python's not adding Client, ping me and I'll look into it.

[lmazuel]

@fearthecowboy I confirm with build 20161212 that it does not work

[olydis]

@fearthecowboy @amarzavery Maybe I get the PR wrong but I see the fix in https://github.com/fearthecowboy/autorest/blob/dae3a099b0e0d90fce04df68f59662f5320347a4/src/generator/AutoRest.CSharp.Azure/CodeNamerCsa.cs#L18

So yes, the code model now consults the code namer, but the Csa one seems to be the only one doing anything smart.

[fearthecowboy]

Ah, yeah, dammit. That's Why.

The change should only affect *Azure generators, so GetClientName has to be overridden in all the CodeNamer??a implementations. (and one has to be created for the ones that don't have one.

sigh

And, now that I think deeper about it, there's a bit more to it.

Leave it for today, If I can recover enough, I'll be in tomorrow, and we can finish solving this right.

[amarzavery]

Why are we trying so hard for this. Right now the storage team is not truly blocked.
If the spec has the following extension: "x-ms-code-generation-settings": { "name": "StorageManagementClient" } then AutoRest will use that name as is. We can do this right now and move ahead. Later we can add a fix to append Client at the right place.

[lmazuel]

@amarzavery but the spec hasn't, so moving to the new spec is not possible for now.

[lmazuel]

@amarzavery we cannot touch the spec without asking the Storage team first, and this is a local workaround (not a bug fix, there is no bug in the spec). And this doesn't change the fact that currently C# code is generating something different than other languages by default.
If we have the opportunity to make it right correctly, we should take it.
My two cents.

[lmazuel]

@amarzavery but as a Azure SDK for Python package manager guy, I agree either way works. As long as Autorest is generating the right client, this side of me is happy :)

[lmazuel]

Any news here? Now the storage package is out, and this is still a problem for Python. Either we use solution from @amarzavery and change the Swagger, or from @fearthecowboy and make Autorest more homogeneous (I prefer that one). But please tell me it's not already forgotten in the Github tracker :)

[fearthecowboy]

Before we jump into anything, let's bring it up for discussion at the next meeting.

[annatisch]

@fearthecowboy, @lmazuel,
So it seems we have concluded to behave consistently across languages, and that ideally this should be moved to the core :)

[fearthecowboy]

Yes, I'll do that.

I'm going to have to add in some sort of azure mode for generators so we can do this in the core when we know it's for Azure.

[lmazuel]

@fearthecowboy any update on this one?

[fearthecowboy]

Sorry, I've been in the middle of XmlSerialization stuff. Lemme see if I can get to it this afternoon.

[lmazuel]

@fearthecowboy No problem :)

[fearthecowboy]

We'll have to work around this in the short term. I'm adding in the Azure-ARM flag when we do configuration, which is coming shortly.

[fearthecowboy]

IIRC this is done.