Azure-rest-api-specs: OperationId in GraphRbac 1.6 should not contains the word Operation

Created on 25 Mar 2016  路  12Comments  路  Source: Azure/azure-rest-api-specs

For instance "operationId": "ApplicationOperations_Create"
Should be "operationId": "Application_Create"

Since the Operations string is already added, this create weird class name with Autorest
image

Not that the operations id were fine in GraphRbac 1.42. Moreover, no other Swagger files use Operations in their operation id.

question

Most helpful comment

In fact, if someone could say exactly what the codenamer should do, I think all the languages could then implement it properly. I'm guessing this is an edge area that is not handled by all.

All 12 comments

yeah... that's a lot of operations... :[

@shuagarw can you provide some feedback on this?

@devigned
There is a collision in the spec with Application being an operation as well as a model definition. To resolve it he added the keyword operation in operationID.

It looks perfectly fine in the node.js codegen https://github.com/Azure/azure-sdk-for-node/tree/master/lib/services/graphManagement/lib/operations

This must be a bug in the python codenamer.

@lmazuel and @amarzavery you guys should probably get on the same page here.

In fact, if someone could say exactly what the codenamer should do, I think all the languages could then implement it properly. I'm guessing this is an edge area that is not handled by all.

@devigned @amarzavery what do you think? It's clearly not just a Python problem. Any change will be a major breaking changes.
I guess we could cut the extra "Operation" suffix if it's necessary for Ruby and Node to have one in Swagger to disambiguate some situations. I'm not a huge fan, I would have preferred that Ruby/Node append the Operations suffix too, like the three other clients. This should be enough to disambiguate in Node.

The ambiguity is in the modeler and not in the Language codenamer.

Sorry, I didn't understood that before.
But it does not change that we have to decide something about the Operations suffix right?

@lmazuel and @amarzavery where does this stand?

After the meeting today and last minute discussion with @devigned and @markcowl we should keep everything the most simple possible with a simple rule:

  • Swagger files should not use Operations in their operation ids
  • Every generator should append Operations. Always.

This keeps the guidance for a language generator really simple that an Operations class must ends with Operations. No language specific rule, no test like if ends with Operations, do not append which will reach a limit one day for sure.
We should _not_ test the presence of Operations in the operation id, the SDK of all languages will be weird enough to force the service team to remove it anyway. This works only if we all follow the same rule.

The RestApi problem is fixed, the Swagger file does not contain "operation" anymore

Was this page helpful?
0 / 5 - 0 ratings