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

added listAccountSas TLE #125

Merged
merged 3 commits into from
Aug 13, 2018
Merged

added listAccountSas TLE #125

merged 3 commits into from
Aug 13, 2018

Conversation

bmoore-msft
Copy link
Contributor

No description provided.

{
"name": "listKeys",
"expectedUsage": "listKeys(resourceName\/resourceIdentifier, apiVersion)",
"expectedUsage": "listKeys(resourceId, apiVersion)",
Copy link
Contributor

Choose a reason for hiding this comment

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

Why these changes? The API specifically lists the argument as resource name or ID:
https://docs.microsoft.com/en-us/azure/azure-resource-manager/resource-group-template-functions-resource#list

Copy link
Contributor Author

Choose a reason for hiding this comment

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

resourceId is correct, resourceName is shorthand. The definitions were inconsistent and I may have over simplified - don't have a strong opinion (other than to be consistent).

Copy link
Contributor

Choose a reason for hiding this comment

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

The question is, can you pass in just a resource name and have it work? If no, stick with resourceId, thanks.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm guessing the test failures are due to the resourceName/Id change. The unit tests are over testing right now - I've loosened it up a bit but not enough. Do you want me to handle fixing them?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I reverted the change (and made the last one consistent) - so that should be back to previous behavior... let's see what happens.

Copy link
Contributor

Choose a reason for hiding this comment

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

So can you actually pass in just the resource name?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

you can - provided that resource is defined in the template... if not, then you need the fulll resourceId.

"maximumArguments": 3,
"returnValueMembers": [
{
"name": "accountSasToken"
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure this is correct. Does it return an object with a accountSasToken property (as you have here), or does it simply return the account SAS token? Thx.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It returns an object with exactly one property named accountSasToken

@StephenWeatherford StephenWeatherford merged commit 672964f into microsoft:master Aug 13, 2018
@StephenWeatherford
Copy link
Contributor

Thanks, Brian.

@bmoore-msft
Copy link
Contributor Author

thx for fixing the tests ;)

@bmoore-msft bmoore-msft deleted the templateexpressions branch August 13, 2018 20:14
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