Skip to content

Comments

Fix update test resources tagging when tags are empty. Bump max limit#7171

Merged
benbp merged 1 commit intoAzure:mainfrom
benbp:benbp/update-timing
Oct 19, 2023
Merged

Fix update test resources tagging when tags are empty. Bump max limit#7171
benbp merged 1 commit intoAzure:mainfrom
benbp:benbp/update-timing

Conversation

@benbp
Copy link
Member

@benbp benbp commented Oct 19, 2023

No description provided.

@benbp benbp added the Central-EngSys This issue is owned by the Engineering System team. label Oct 19, 2023
@benbp benbp self-assigned this Oct 19, 2023
@azure-sdk
Copy link
Collaborator

The following pipelines have been queued for testing:
java - template
java - template - tests
js - template
net - template
net - template - tests
python - template
python - template - tests
You can sign off on the approval gate to test the release stage of each pipeline.
See eng/common workflow

Log "Updating DeleteAfter to '$deleteAfter'"
Write-Warning "Any clean-up scripts running against subscription '$SubscriptionId' may delete resource group '$ResourceGroupName' after $DeleteAfterHours hours."
if (!$resourceGroup.Tags) {
$resourceGroup.Tags = @{}
Copy link
Member

Choose a reason for hiding this comment

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

This is actually necessary? Seems more like a bug on the Az PowerShell module. As a workaround, though, I see no harm but if it is a bug, we should make sure it's reported. PowerShell is truthy, so idiomatically so should cmdlets be.

Copy link
Member Author

Choose a reason for hiding this comment

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

It's all json at the API level so I'm not surprised if the underlying value was null that powershell just converted it to $null without initializing empty values?

Copy link
Member

Choose a reason for hiding this comment

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

That's my assumption as well...but should. If the value is falsy, it should omit the tags in the request or pass an empty object - whatever the control plane requires in that case.

Copy link
Member Author

Choose a reason for hiding this comment

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

The issue here is we try to index into $null a line below, not that we're submitting bad API data.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, I see. Thanks for the clarification. That said, does sending whatever @{} serializes into cause problems?

Copy link
Member Author

Choose a reason for hiding this comment

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

It worked when I tested it out. The response object is also a hashtable:

$ $g[0].Tags.GetType()

IsPublic IsSerial Name                                     BaseType
-------- -------- ----                                     --------
True     True     Hashtable                                System.Object

@benbp benbp enabled auto-merge (squash) October 19, 2023 22:58
azure-sdk added a commit to Azure/azure-sdk-for-js that referenced this pull request Oct 19, 2023
Sync eng/common directory with azure-sdk-tools for PR
Azure/azure-sdk-tools#7171 See [eng/common
workflow](https://github.com/Azure/azure-sdk-tools/blob/main/eng/common/README.md#workflow)

Co-authored-by: Ben Broderick Phillips <bebroder@microsoft.com>
@benbp benbp merged commit e227149 into Azure:main Oct 19, 2023
@benbp benbp deleted the benbp/update-timing branch October 19, 2023 23:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Central-EngSys This issue is owned by the Engineering System team.

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

3 participants