Skip to content

Conversation

@pieandcakes
Copy link

InsertJsonValues can be too long if there are many vsman to insert. If this happens, AzDO has a variable limitation of ~32k characters for the variable. This adds a check to use the newly introduced InsertJsonValuesFile where the InsertJsonValues is written to a file and then processed by InsertVSPayload.

InsertJsonValues can be too long if there are many vsman to insert. If
this happens, AzDO has a variable limitation of ~32k characters for the
variable. This adds a check to use the newly introduced
InsertJsonValuesFile where the InsertJsonValues is written to a file
and then processed by InsertVSPayload.
# write the value to a temp file and use the InsertJsonValuesFile variable that InsertVSPayload understands.
if ($_.BaseName -eq "InsertJsonValues") {
Write-Warning "The script InsertJsonValues.ps1 is longer than 32k characters."
Write-Warning "Writing to a temp file and setting INSERTJSONVALUESFILE to the location of the file."
Copy link
Owner

Choose a reason for hiding this comment

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

Rather than special case InsertJsonValues in this script (which should be general purpose), I think converting the value to a .txt file should be done at insertion time where it has to deal with this variable specifically.

Copy link
Author

Choose a reason for hiding this comment

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

So you're saying that the check (and converting it to a text file) should be in _define.ps1 ?

# Write file to the new parameter.
$vars["InsertJsonValuesFile"] = "$tmpFile"
} else {
Write-Error "The script $($_.BaseName).ps1 is too long (> 32k characters). Skipping setting the environment variable."
Copy link
Owner

Choose a reason for hiding this comment

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

I think this should be a warning rather than an error.

Copy link
Owner

@AArnott AArnott left a comment

Choose a reason for hiding this comment

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

When the changes I've requested are applied, I think this PR should target the main branch, and a separate PR should that targets microbuild should edit the insertion pipeline to handle the long variable value specially.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants