Skip to content

Add UserMetricsModel to exception list for missing readOnly attribute. #4003

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

Merged
dsgouda merged 1 commit into from
Sep 26, 2018
Merged

Add UserMetricsModel to exception list for missing readOnly attribute. #4003

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions specification/trafficmanager/resource-manager/readme.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,10 @@ directive:
reason: This will cause breaking changes in .NET SDK
from: trafficmanager.json
where: $.definitions.Endpoint
- suppress: RequiredPropertiesMissingInResourceModel
reason: This will cause breaking changes in .NET SDK
from: trafficmanager.json
where: $.definitions.UserMetricsModel
Copy link
Member

@anuchandy anuchandy Sep 26, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tatlicioglu - this is to suppress the following ARMViolation I guess:

RequiredPropertiesMissingInResourceModel/R2020/ARMViolation: Model definition 'UserMetricsModel' must have the properties 'name', 'id' and 'type' in its hierarchy and these properties must be marked as readonly.

Looking at the definition of UserMetricsModel it does have name, id and type in the hierarchy (UserMetricsModel -> ProxyResource -> Resource), but id is not readonly.

As mentioned in the suppression reason, adding readonly attribute for Resource::id will cause breaking change in the SDK (e.g. ctr for UserMetricsModel will break) that requires a major version bump for the SDK. A major version upgrade of SDK for such a small change doesn't worth.

So we might want add the work of marking id as readonly as a TODO in major version upgrade work items. At this point i'm not sure when that happens. Maybe we should introduce a process of cleaning the swaggers once in a semester or so where we fix all these issues and do a major version upgrade. Something that needs to be discussed within the team.

It looks like there are bunch of other models (Endpoint, Profile, TrafficManagerGeographicHierarchy & HeatMapModel) inherits from Resource has RequiredPropertiesMissingInResourceModel. Any reason not to suppress violations for those models?

\cc @johanste, @salameer

```

### Tag: package-2018-04
Expand Down