Azure / typespec-azure

About TypeSpec Azure Libraries
https://azure.github.io/typespec-azure/
MIT License
9 stars 29 forks source link

Question for `resourceUri` parameter of extension resource #1022

Closed tadelesh closed 2 weeks ago

tadelesh commented 1 month ago

The definition is as follows:

@doc("The default resourceUri parameter type.")
model ResourceUriParameter {
  @path
  @doc("The fully qualified Azure Resource manager identifier of the resource.")
  @extension("x-ms-skip-url-encoding", true)
  @resourceParameterBaseFor([ResourceHome.Extension])
  resourceUri: string;
}

How can emitter know the resourceUri does not need url encoding since it is not typed with url? IDK whether url type is by design for that?

tadelesh commented 1 month ago

blocking scvmm release.

I am reading from extension in emitter for .NET

reading openapi extension info is not a good solution.

MaryGao commented 1 month ago

@timotheeguerin I heard that we have assumption that for type url it would implicitly imply true for x-ms-skip-url-encoding and with type string it would imply false. Is this correct?

Personally I don't think url vs string type would imply any encoding info.

tadelesh commented 1 month ago

found some previous discussion but not fully understood the final decision: https://github.com/microsoft/typespec/issues/851. @markcowl could you also help with this question?

ArcturusZhang commented 3 weeks ago

why not we just model the type of resourceUri to armResourceIdentifier? In practice, this place is always an armResourceIdentifier. And we could add the skip-url-encoding extension to the definition of armResourceIdentifier because when an arm id was put in path, url encoding should always be skipped.

For this perspective, maybe it is wrong that we defined armResourceIdentifier to extends string. Maybe it should extends url

ArcturusZhang commented 3 weeks ago

Also there is a requirement from dotnet MPG generator that this type is an arm id. Could we also rename the parameter to be resourceId maybe to be more specific? Because it is an arm id in ARM standard and practice.

tadelesh commented 3 weeks ago

the difficulty here is to be backward compatible with swagger: https://github.com/Azure/azure-rest-api-specs/blob/87a08b955c257c773a3bd42553c718d4b1092955/specification/common-types/resource-management/v6/types.json#L627

timotheeguerin commented 2 weeks ago

will close in favor of hte core design issue for this https://github.com/microsoft/typespec/issues/3736