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

Resource model changes the order of properties #1979

Open
Tracked by #1850
pshao25 opened this issue Dec 12, 2024 · 3 comments
Open
Tracked by #1850

Resource model changes the order of properties #1979

pshao25 opened this issue Dec 12, 2024 · 3 comments
Assignees

Comments

@pshao25
Copy link
Member

pshao25 commented Dec 12, 2024

Think about this case:

"experiment": {
  "type": "object",
  "description": "Model that represents a Experiment resource.",
  "allOf": [
    {
      "$ref": "../../../../../../common-types/resource-management/v2/types.json#/definitions/TrackedResource"
    }
  ],
  "properties": {
    "systemData": {
      "description": "The system metadata of the experiment resource.",
      "$ref": "../../../../../../common-types/resource-management/v2/types.json#/definitions/systemData",
      "readOnly": true
    },
    "identity": {
      "description": "The identity of the experiment resource.",
      "$ref": "./common.json#/definitions/resourceIdentity"
    },
    "properties": {
      "description": "The properties of the experiment resource.",
      "x-ms-client-flatten": true,
      "$ref": "#/definitions/experimentProperties"
    }
  },
  "required": [
    "properties"
  ]
}

identity is in front of properties.

Transform it into TypeSpec:

model Experiment
  is Azure.ResourceManager.TrackedResource<ExperimentProperties, false> {
  ...ResourceNameParameter<
    Resource = Experiment,
    KeyName = "experimentName",
    SegmentName = "experiments",
    NamePattern = "^[^<>%&:?#/\\\\]+$"
  >;
  ...Azure.ResourceManager.ManagedServiceIdentityProperty;
}

identity goes after properties.

Change the order of properties is breaking change in .net SDK.

Though we might somehow do customization to mitigate,

  1. It looks so common. We don't want to do so many customizations.
  2. If there is a new property added in XXXProperties (in this case, customer does add a new property to ExperimentProperties), it would be a disaster to mitigate.
  3. Further, if identity is type of string, experimentProperties contains xxx: string. the order changes from (string identity, string xxx) to (string xxx, string identity). There is no solution to customize.
@markcowl
Copy link
Member

markcowl commented Dec 16, 2024

If the order of properties is a critical issue, then the template is not required for this scenario:

model Experiment extends Core.TrackedResource {
  ...ResourceNameParameter<
    Resource = Experiment,
    KeyName = "experimentName",
    SegmentName = "experiments",
    NamePattern = "^[^<>%&:?#/\\\\]+$"
  >;
 
  ...Azure.ResourceManager.ManagedServiceIdentityProperty;
  properties?: ExperimentProperties;
}

Using a template is not required.

@brrusino
Copy link
Member

brrusino commented Dec 17, 2024

@markcowl After making this change, I am getting the following error:
No @armResource registration found for type Experiment

Also, which Core library is this TrackedResource model in? I was able to find it in Azure.ResourceManager.Foundations but not Azure.Core.

@brrusino brrusino reopened this Dec 17, 2024
@pshao25
Copy link
Member Author

pshao25 commented Dec 18, 2024

More specifically for this case, you should do

/**
 * Model that represents a Experiment resource.
 */
@Azure.ResourceManager.Private.armResourceInternal(ExperimentProperties)
@includeInapplicableMetadataInPayload(false)
model Experiment extends Foundations.TrackedResource {
  ...ResourceNameParameter<
    Resource = Experiment,
    KeyName = "experimentName",
    SegmentName = "experiments",
    NamePattern = "^[^<>%&:?#/\\\\]+$"
  >;
  ...Azure.ResourceManager.ManagedServiceIdentityProperty;

  @doc("The resource-specific properties for this resource.")
  @Azure.ResourceManager.Private.conditionalClientFlatten
  @Azure.ResourceManager.Private.armResourcePropertiesOptionality(false)
  properties?: ExperimentProperties;
}

Then suppress all the warnings because of the private decorators.

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

No branches or pull requests

3 participants