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

Resolve bug in API View Script #31637

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Changes from 1 commit
Commits
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
38 changes: 23 additions & 15 deletions eng/scripts/Create-APIView.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,19 @@ function Get-SwaggerReadMeFile {
)

$currentPath = Resolve-Path $SwaggerFile

while ($currentPath -ne [System.IO.Path]::GetPathRoot($currentPath)) {
$currentPath = [System.IO.Path]::GetDirectoryName($currentPath)
$readmeFile = Get-ChildItem -Path $currentPath -Filter "readme.md" -File -ErrorAction SilentlyContinue
if ($readmeFile -and $readmeFile.Name -eq "readme.md") {
return $readmeFile.FullName
}
$currentPath = [System.IO.Path]::GetDirectoryName($currentPath)
$currentFilePath = [System.IO.Path]::GetFileName($currentPath)

if ($currentFilePath -eq "specification") {
break
}

$readmeFile = Get-ChildItem -Path $currentPath -Filter "readme.md" -File -ErrorAction SilentlyContinue
if ($readmeFile -and $readmeFile.Name -eq "readme.md") {
return $readmeFile.FullName
}
}

return $null
Expand Down Expand Up @@ -111,15 +117,17 @@ function Invoke-SwaggerAPIViewParser {
LogGroupEnd

$generatedAPIViewTokenFile = Get-ChildItem -File | Select-Object -First 1
Copy link
Member

Choose a reason for hiding this comment

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

Do we not know the expected outputted file name?

Copy link
Member

Choose a reason for hiding this comment

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

Is the file named after the tag? If so is that a hacky way of getting the tag name as output or does it make since for it to be named that and we should get that value another way?

Copy link
Member Author

@chidozieononiwu chidozieononiwu Nov 21, 2024

Choose a reason for hiding this comment

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

I have added logic to get the tag on the pipeline side so I can pass the tag to the tool rather than relying on the tool to find the default tag.

$readMeTag = $generatedAPIViewTokenFile.BaseName

LogSuccess " Generated '$Type' APIView Token File using file, '$readMeFile' and tag '$readMeTag'"

$apiViewTokensFilePath = [System.IO.Path]::Combine($TokenDirectory, "$resourceProvider.$Type.json")
LogInfo " Moving generated APIView Token file to '$apiViewTokensFilePath'"
Move-Item -Path $generatedAPIViewTokenFile.FullName -Destination $apiViewTokensFilePath -Force > $null
if (Test-Path -Path $generatedAPIViewTokenFile) {
Copy link
Member

Choose a reason for hiding this comment

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

If this file doesn't exist I would assume the parser failed. Wouldn't we have a failing exit code to look at in that case?

Copy link
Member Author

@chidozieononiwu chidozieononiwu Nov 21, 2024

Choose a reason for hiding this comment

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

No necessarily. In the case where the tag is not found in the readme.md it will not create any APIView token but it will not be a failure.

$readMeTag = $generatedAPIViewTokenFile.BaseName

return $readMeTag
LogSuccess " Generated '$Type' APIView Token File using file, '$readMeFile' and tag '$readMeTag'"

$apiViewTokensFilePath = [System.IO.Path]::Combine($TokenDirectory, "$resourceProvider.$Type.json")
LogInfo " Moving generated APIView Token file to '$apiViewTokensFilePath'"
Move-Item -Path $generatedAPIViewTokenFile.FullName -Destination $apiViewTokensFilePath -Force > $null
return $readMeTag
}
} catch {
LogError " Failed to generate '$Type' APIView Tokens using '$readMeFile' for '$resourceProvider'"
throw
Expand Down Expand Up @@ -183,12 +191,12 @@ function New-SwaggerAPIViewTokens {
$changedSwaggerFiles | ForEach-Object {
$readmeFile = Get-SwaggerReadMeFile -swaggerFile $_
if ($readmeFile) {
$swaggerReadMeFiles.Add($readmeFile) | Out-Null
$swaggerReadMeFiles.Add($readmeFile) | Out-Null
}
}

LogGroupStart " Swagger APIView Tokens will be generated for the following configuration files..."
$readmeFile | ForEach-Object {
$swaggerReadMeFiles | ForEach-Object {
LogInfo " - $_"
}
LogGroupEnd
Copy link
Member

Choose a reason for hiding this comment

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

Below are we assuming the that default tag name doesn't change between the source and the target? What if they change the tag in the PR?

Copy link
Member Author

@chidozieononiwu chidozieononiwu Nov 21, 2024

Choose a reason for hiding this comment

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

The APIView is first created from the PR source branch using the default tag. Then a baseline is created on the target branch using the same tag if the tag is present in the PR. If not, no baseline is created.

Copy link
Member Author

Choose a reason for hiding this comment

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

I have updated the logic to revert to using the default tag if the new tag (source branch) is not found on the target branch

Expand Down