Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

track1 code generator is generating weird method name for arm-policyinsights #1261

Closed
qiaozha opened this issue Dec 10, 2021 · 10 comments
Closed
Assignees
Labels

Comments

@qiaozha
Copy link
Member

qiaozha commented Dec 10, 2021

We are trying to generate the mgmt plane track1 sdk for policyinsights RP. The command we are using is
autorest --typescript --typescript-sdks-folder=../azure-sdk-for-js --license-header=MICROSOFT_MIT_NO_VERSION ../azure-rest-api-specs/specification/postgresql/resource-manager/readme.md [email protected]/[email protected]

And we get some weird method names like nextLink1, nextLink2, nextLink3 etc in sdk/policyinsights/arm-policyinsights/src/operations/policyStates.ts
image

we also tried to use @microsoft.azure/[email protected] and @microsoft.azure/[email protected] still not working

--version:2.0.4417 also doesn't work.

@sarangan12
Copy link
Contributor

@qiaozha The title and description mentions policyInsights RP. But, the command references the file ../azure-rest-api-specs/specification/postgresql/resource-manager/readme.md which is related to postgresql. Is the command added by mistake?

I am going to try both the SDKs and update this issue.

@sarangan12
Copy link
Contributor

First, I have tried the postgresql SDK using the following command:

autorest --typescript --typescript-sdks-folder=C:\Users\sarajama\Projects\azure-sdk-for-js --license-header=MICROSOFT_MIT_NO_VERSION [email protected]/[email protected] --title=PostgreSQLFlexibleManagementClient C:\Users\sarajama\Projects\azure-rest-api-specs\specification\postgresql\resource-manager\readme.md
SourceMapConsumer.initialize is a no-op when running in node.js
AutoRest code generation utility [cli version: 3.5.1; node: v14.15.4]
(C) 2018 Microsoft Corporation.
https://aka.ms/autorest
info    | AutoRest core version selected from configuration: ~2.0.4413.
   Loading AutoRest core      'C:\Users\sarajama\.autorest\@[email protected]\nodemodules\@microsoft.azure\autorest-core\dist' (2.0.4421)
   Loading AutoRest extension '@microsoft.azure/autorest.typescript' (4.7.0->4.7.0)
   Loading AutoRest extension '@microsoft.azure/autorest.modeler' (2.3.51->2.3.51)
INFORMATION: Getting the package version from the pre-existing package.json file in the output folder.
INFORMATION: Got version "1.0.0" for package "@azure/arm-postgresql-flexible" from existing package.json file.

The generation was successful. Build was successful. There is no reference to any names such as nextLink2, `nextLink3', etc. So, there is no problem with postgresql.

I am going to try the policyInsights and update the issue.

@sarangan12
Copy link
Contributor

Used the following command to generate policyInsights RP:

autorest --typescript --typescript-sdks-folder=C:\Users\sarajama\Projects\azure-sdk-for-js --license-header=MICROSOFT_MIT_NO_VERSION [email protected]/[email protected]  C:\Users\sarajama\Projects\azure-rest-api-specs\specification\policyinsights\resource-manager\readme.md
SourceMapConsumer.initialize is a no-op when running in node.js
AutoRest code generation utility [cli version: 3.5.1; node: v14.15.4]
(C) 2018 Microsoft Corporation.
https://aka.ms/autorest
info    | AutoRest core version selected from configuration: ~2.0.4413.
   Loading AutoRest core      'C:\Users\sarajama\.autorest\@[email protected]\nodemodules\@microsoft.azure\autorest-core\dist' (2.0.4421)
   Loading AutoRest extension '@microsoft.azure/autorest.typescript' (4.7.0->4.7.0)
   Loading AutoRest extension '@microsoft.azure/autorest.modeler' (2.3.51->2.3.51)
INFORMATION: Getting the package version from the pre-existing package.json file in the output folder.
INFORMATION: Got version "4.0.0" for package "@azure/arm-policyinsights" from existing package.json file

I was able to generate and build the SDK. But, I see the nextLink, nextLink2, nextLink3 in the generated SDK and this has to be fixed. Let me work on it and update this issue.

@sarangan12
Copy link
Contributor

sarangan12 commented Dec 10, 2021

I have checked the generated SDK and the swagger provided. Here, this issue started happening from commit id: b189836eb664ef8d660ccf761a4111bee19ad840 which was committed on May 24, 2021 in this PR: Azure/azure-rest-api-specs#14035

Let me give an example. Take a look at this file. The operation id is PolicyStates_ListQueryResultsForManagementGroup. But, with the above mentioned commit id, the following line is added:

"x-ms-pageable": {
  "nextLinkName": "@odata.nextLink",
  "operationName": "PolicyStates_NextLink"
}

As you can see, the operation name is overridden and set to NextLink. This is happening at 9 places in the same file. which resulted in names such as nextLink, nextLink1, etc.

Now, why was this done? I think service team would be the best to answer that question.

How do we solve this?
I have checked the Java SDK for policyInsights RP. To the best of my understanding, Java SDK does not follow the JS Logic. So, they do not have this problem. So, our generator might have this problem. But, since this is a old version and possibly be out of date in the next few months (@ramya-rao-a Please share your thoughts) I would not go ahead with code fix here.

Instead, please do a swagger transform that overrides the operationName which will solve your problem.

Please let me know if this solves your problem or if you need any more details. Thanks

@ramya-rao-a
Copy link
Contributor

If a swagger transform can help here, I would recommend that route given that this issue is for the older generator.

Can we check whether the new generator will have the same problem?

@sarangan12
Copy link
Contributor

sarangan12 commented Dec 11, 2021

@ramya-rao-a The error does not happen with the latest generator (However this SDK needs leniant model dupliction flag, which is not unrelated)

@qiaozha
Copy link
Member Author

qiaozha commented Dec 13, 2021

@sarangan12 Thanks for such detailed investigation here, let me check with service team about this and get back to you. I think we can hold on the fix for it here until we get more information from service team.

@sarangan12
Copy link
Contributor

@qiaozha is there any update from the service team?

@sarangan12
Copy link
Contributor

@qiaozha As there are no pending action items, I am closing this task. If you find anything new from the Service Team and would like to update the generator, feel free to reopen this task.

@qiaozha
Copy link
Member Author

qiaozha commented Mar 3, 2022

@qiaozha As there are no pending action items, I am closing this task. If you find anything new from the Service Team and would like to update the generator, feel free to reopen this task.

Yes, it's okay to close it, We decide to release track2 SDK directly for this RP which works as expected. Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants