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.

Sign up for GitHub

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

Jump to bottom

Operation ID changed after tsp 0.61 #5117

Open
archerzz opened this issue Oct 17, 2024 · 2 comments · May be fixed by #5210
Open

Operation ID changed after tsp 0.61 #5117

archerzz opened this issue Oct 17, 2024 · 2 comments · May be fixed by #5210
Assignees
@archerzz
Labels
Mgmt This issue is related to a management-plane library. v3 Version 3 of AutoRest C# generator.

Comments

@archerzz
Copy link
Member

Description

In tsp 0.60 and prior, getResourceOperation will return undefined for some mgmt cases, like

autorest.csharp/test/TestProjects/MgmtTypeSpec/foo.tsp

Lines 10 to 15 in ab56c7d

@resource("foos")
model Foo is TrackedResource<FooProperties> {
...ResourceNameParameter<Foo, SegmentName = "foos">;
...ExtendedLocationProperty;
}

Image

So, previous the ResourceName is Foos which is the interface name:

autorest.csharp/test/TestProjects/MgmtTypeSpec/foo.tsp

Line 36 in ab56c7d

interface Foos {

But in 0.61, getResourceOperation can return the resource operation, which seems the correct behavior. However, that will return different ResourceName. Check this PR: https://github.com/Azure/autorest.csharp/pull/5110/files#diff-81dc15d9aacc476e7f1fc431451c064ac2727e5c09cde422e3ddc362b7b57981L2714

I think the upstream actually has fixed an issue, but unfortunately cause regression in some cases.
@archerzz archerzz added Mgmt This issue is related to a management-plane library. v3 Version 3 of AutoRest C# generator. labels Oct 17, 2024
@archerzz archerzz mentioned this issue Oct 17, 2024
Merged
6 tasks
archerzz pushed a commit to archerzz/typespec that referenced this issue Jan 3, 2025
feat(http-client-csharp): add InputOperation.OperationId
45c7044 
- add `InputOperation.OperationId`
- invoke `resolveOperationId()` to initialize it

part of Azure/autorest.csharp#5117
@archerzz archerzz mentioned this issue Jan 3, 2025
Draft
archerzz pushed a commit to archerzz/typespec that referenced this issue Jan 3, 2025
feat(http-client-csharp): add InputOperation.OperationId
994d614 
- add `InputOperation.OperationId`
- invoke `resolveOperationId()` to initialize it

part of Azure/autorest.csharp#5117
archerzz pushed a commit to archerzz/typespec that referenced this issue Jan 3, 2025
feat(http-client-csharp): add InputOperation.OperationId
de848fc 
- add `InputOperation.OperationId`
- invoke `resolveOperationId()` to initialize it

part of Azure/autorest.csharp#5117
archerzz pushed a commit to archerzz/autorest.csharp that referenced this issue Jan 3, 2025
feat(generator): use InputOperation.OperationId
765d680 
Previously we use resource name to guess operation id, which doesn't work well in some cases.
Instead of gussing operation id in generator, we try best to get the id in emitter and pass it to generator.

resolve Azure#5117
This was referenced Jan 3, 2025
Draft
Closed
@archerzz
Copy link
Member Author

archerzz commented Jan 7, 2025

The impact of this change is non-trivial. See the regen preview: Azure/azure-sdk-for-net#47714

@archerzz
Copy link
Member Author

archerzz commented Jan 8, 2025

After trial and discussion, we thought we could choose to not do this item due to:

  • impact of the change
  • operation id should soon be history, probably not worth the efforts

We have 3 types of inputs right now. And for each of them, there is a corresponding issue regarding operation id:

  1. Swagger: we lose the original operation id during CodeModelConverter. Most of the changes in the regen preview PR are from that. Moreover, since we fix the wrong operation id, we have to fix the wrong configuration in autorest.md. Some configurations use operation id as the key.
  2. Pure TypeSpec: there is no operation id in TypeSpec, and we need to figure out how we want to resolve it.
  3. TypeSpec converted from Swagger: the way we resolve operation id in TypeSpec will have impact on how we convert Swagger to TypeSpec, so as to keep operation id consistent.

Image

All in all, if we want to fix the operation id, we must have InputOperation.OperationId passed in from Swagger or from TypeSpec emitter, instead of deducing it in the generator. Previously we have discussed about it, and we thought we don't want to do that.

Since there is no obvious negative outcome, maybe we can choose to not do this item.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@archerzz archerzz

Labels
Mgmt This issue is related to a management-plane library. v3 Version 3 of AutoRest C# generator.
Projects
None yet
Milestone
No milestone
1 participant
@archerzz