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

Update contract for anomaly detector #12487

Merged
merged 13 commits into from
Feb 9, 2021
Merged

Update contract for anomaly detector #12487

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
df9d08d
update contract for anomaly detector
Jan 19, 2021
696a311
make severity optional
Jan 19, 2021
6913db3
contract update
Jan 20, 2021
a7fc832
update according to PR
Jan 21, 2021
9d71cd0
update values
Jan 22, 2021
37f0a8c
update
Jan 22, 2021
ea9e4a6
prettier fix
Jan 26, 2021
2b8023e
split the PR into two parts, this pr adds two enum values into the gr…
Feb 5, 2021
bc8261b
change preview to stable
Feb 5, 2021
98e62a3
update to stable version
Feb 7, 2021
d61813c
fix build failure
Feb 8, 2021
687a6fb
update
Feb 9, 2021
b86618a
update
Feb 9, 2021
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
79 changes: 76 additions & 3 deletions specification/cognitiveservices/data-plane/AnomalyDetector/preview/v1.0/AnomalyDetector.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@
},
"TimeGranularity": {
"type": "string",
"description": "Can only be one of yearly, monthly, weekly, daily, hourly, minutely or secondly. Granularity is used for verify whether input series is valid.",
"description": "Optional argument, can be one of yearly, monthly, weekly, daily, hourly, minutely, secondly or microsecond. If granularity is present in the request, it will be used to verify whether input series is valid and to impute missing values.",
"x-nullable": false,
"x-ms-enum": {
"name": "TimeGranularity",
Expand Down Expand Up @@ -226,9 +226,65 @@
"daily",
"hourly",
"minutely",
"secondly"
"secondly",
"microsecond"
jhendrixMSFT marked this conversation as resolved.
Show resolved Hide resolved
]
},
"ImputeMode": {
"type": "string",
"description": "can be one of auto, previous, linear, fixed, zero or notFill.",
guinao marked this conversation as resolved.
Show resolved Hide resolved
"x-nullable": false,
"x-ms-enum": {
"name": "TimeGranularity",
guinao marked this conversation as resolved.
Show resolved Hide resolved
"modelAsString": false,
"values": [
{
"value": "auto"
},
{
"value": "previous"
},
{
"value": "linear"
},
{
"value": "fixed"
},
{
"value": "zero"
},
{
"value": "notFill"
}
]
},
"enum": [
"auto",
"previous",
"linear",
"fixed",
"zero",
"notFill"
]
},
"ImputeStrategy": {
"type": "object",
"required": [
"imputeMode"
],
"properties": {
"imputeMode": {
"$ref": "#/definitions/ImputeMode",
"description": "The method that is used to impute missing values in the given time series."
},
"imputeValue": {
"type": "number",
"format":"float",
"x-nullable": false,
"description": "Optional argument, if imputeMode is set to 'fixed', then this value is used to impute missing values."
}
}
},
"CustomInterval": {
"type": "integer",
"format": "int32",
Expand All @@ -238,7 +294,6 @@
"DetectRequest": {
"type": "object",
"required": [
"granularity",
guinao marked this conversation as resolved.
Show resolved Hide resolved
"series"
],
"properties": {
Expand Down Expand Up @@ -270,6 +325,10 @@
"type": "integer",
"format": "int32",
"description": "Optional argument, advanced model parameter, between 0-99, the lower the value is, the larger the margin value will be which means less anomalies will be accepted."
},
"imputeStrategy": {
"$ref":"#/definitions/ImputeStrategy",
"description": "Optional argument, indicates how to impute the missing values in the series."
}
}
},
Expand Down Expand Up @@ -359,6 +418,15 @@
"type": "boolean",
"x-nullable": false
}
},
"severity": {
"type": "array",
Copy link
Contributor

Choose a reason for hiding this comment

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

If this is a value returned by your service, consider marking it as readOnly: true

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should other fileds also be marked as readOnly as well?

In reply to: 560688590 [](ancestors = 560688590)

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, if they are set by your service, see here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

According to the following rule, I changed the property of our output model object to readOnly and removed the 'required' property.

R2056 RequiredReadOnlyProperties
Category : SDK Error

Applies to : ARM and Data plane OpenAPI(swagger) specs

Output Message: Property '{0}' is a required property. It should not be marked as 'readonly'.

Description: A model property cannot be both readOnly and required. A readOnly property is something that the server sets when returning the model object while required is a property to be set when sending it as a part of the request body.

Why the rule is important: SDK generation fails when this rule is violated.

How to fix the violation: Ensure that the given property is either marked as readonly: true or required but not both.

Copy link
Contributor

Choose a reason for hiding this comment

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

Correct, if a customer sets the value, then it cannot be readonly (you can enforce setting the value by specifying required but nothing else), readonly: true is set on properties that your service sets an example is ProvisioningState, this value is set by your service (Running, Provisioning, Succeeded | Failed, etc.) so just add readonly: true to those properties that your service sets.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks just updated the PR.

"description": "Severity contains the severity score for each input point. For a normal point, the severity score is 0. For an anomaly point, the severity score is in (0, 1] that indicates the severity of the anomaly.",
"items": {
"type": "number",
"format": "float",
"x-nullable": false
}
}
}
},
Expand Down Expand Up @@ -411,6 +479,11 @@
"isPositiveAnomaly": {
"type": "boolean",
"description": "Anomaly status in positive direction of the latest point. True means the latest point is an anomaly and its real value is larger than the expected one."
},
"severity": {
Copy link
Contributor

Choose a reason for hiding this comment

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

Your service is in preview, but consider marking properties as readOnly where needed, follow this doc.

Copy link
Contributor Author

@guinao guinao Jan 22, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Actually our service is going to GA. We just have another PR to change 'preview' to 'stable' #12472.

"type": "number",
"format": "float",
"description": "Severity score of the latest point. For a normal point, the severity score is 0. For an anomaly point, the severity score is in (0, 1] that indicates the severity of the anomaly."
}
}
},
Expand Down
56 changes: 55 additions & 1 deletion ...tion/cognitiveservices/data-plane/AnomalyDetector/preview/v1.0/examples/EntireDetect.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,11 @@
],
"maxAnomalyRatio": 0.25,
"sensitivity": 95,
"granularity": "monthly"
"granularity": "monthly",
"imputeStrategy": {
"imputeMode": "fixed",
"imputeValue": 850
}
}
},
"responses": {
Expand Down Expand Up @@ -506,6 +510,56 @@
false,
false,
false
],
"severity": [
0.0,
0.0,
0.0,
0.0,
0.0,
0.0,
0.0,
0.0,
0.0,
0.0,
0.0,
0.0,
0.0,
0.0,
0.0,
0.0,
0.0,
0.0,
0.0,
0.0,
0.0,
0.0,
0.0,
0.0,
0.0,
0.2906614447614368,
0.0,
0.0,
0.0,
0.0,
0.0,
0.0,
0.0,
0.0,
0.0,
0.0,
0.0,
0.0,
0.0,
0.0,
0.0,
0.0,
0.0,
0.0,
0.0,
0.0,
0.0,
0.0
]
}
}
Expand Down
7 changes: 6 additions & 1 deletion ...cation/cognitiveservices/data-plane/AnomalyDetector/preview/v1.0/examples/LastDetect.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,11 @@
],
"maxAnomalyRatio": 0.25,
"sensitivity": 95,
"granularity": "monthly"
"granularity": "monthly",
"imputeStrategy": {
"imputeMode": "fixed",
"imputeValue": 850
}
}
},
"responses": {
Expand All @@ -213,6 +217,7 @@
"expectedValue": 809.23280846597038,
"upperMargin": 40.461640423298519,
"lowerMargin": 40.461640423298519,
"severity": 0.0,
"suggestedWindow": 49
}
}
Expand Down