[TypeSpecRequirement] Only allow one version per suppression #29001
Conversation
|
PR validation pipeline restarted successfully. This comment will be populated with next steps to merge this PR once validation is completed. Please wait ⌛. |
Swagger Validation Report
|
Swagger Generation Artifacts
|
|
PR validation pipeline restarted successfully. If there is ApiView generated, it will be updated in this comment. |
| @@ -20,9 +20,27 @@ function Get-Suppression { | |||
|
|
|||
| # -NoEnumerate to prevent single-element arrays from being collapsed to a single object | |||
| # -AsHashtable is closer to raw JSON than PSCustomObject | |||
| $suppressions = npx get-suppressions TypeSpecRequirement $fileInSpecFolder | ConvertFrom-Json -NoEnumerate -AsHashtable | |||
| $suppressions = npm exec --no -- get-suppressions TypeSpecRequirement $fileInSpecFolder | ConvertFrom-Json -NoEnumerate -AsHashtable | |||
There was a problem hiding this comment.
Unrelated to crux of PR. npm exec --no -- is more predictable in scripts than npx and should always be used instead.
There was a problem hiding this comment.
The TypeSpec Requirements pipeline was not calling the npm install step so this was never working after your change on friday. And the last error check you added below started to cause the pipeline to fail which was good because we started to see that failure now.
I've added the npm install step to the pipeline in PR #29008
There was a problem hiding this comment.
I expected $suppressions = npm exec to throw by default if the command returned a non-zero exit code. I was surprised we need to manually check $LASTEXITCODE.
|
|
||
| return $suppressions ? $suppressions[0] : $null; | ||
| if ($LASTEXITCODE -ne 0) { |
There was a problem hiding this comment.
Unrelated to crux of PR. But if get-suppressions fails, either because user didn't run npm ci and tool isn't compiled, or because say suppressions.yaml is malformed and get-suppressions throws, we should fail fast.
| if ($suppression) { | ||
| $path = $suppression["path"] | ||
| $singleVersionPattern = "/(preview|stable)/[A-Za-z0-9._-]+/" | ||
| if ($path -notmatch $singleVersionPattern) { |
There was a problem hiding this comment.
Crux of PR. Require that paths specified in suppressions.yaml for this tool only map to a single API version. Suppressions for additional versions should require additional entries and reviews.
Handles only exisiting suppression https://github.com/Azure/azure-rest-api-specs/blob/main/specification/oracle/suppressions.yaml