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

Expand ARM output into Pipeline variables #13811

Merged

Conversation

TomBonnerAtTFL
Copy link
Contributor

Hi there Microsoft! I've added code that expands the returned Outputs object into Pipeline variables so they can be used easily in scripts. This fixes issue #13032.

Task name: AzureResourceGroupDeploymentV2

Description: Added code to expand the Outputs into more accessible variables

Documentation changes required: Yes

Added unit tests: No

Attached related issue: Original issue can be found here.

Checklist:

  • Task version was bumped - please check instruction how to do it
  • Checked that applied changes work as expected

Added code that expands the returned Outputs object into Pipeline variables so they can be used easily in scripts. This fixes issue microsoft#13032.
@TomBonnerAtTFL
Copy link
Contributor Author

This gist of it is that it's difficult to work with ARM template outputs because they're wrapped up as a JSON object. This means you can to constantly unwrap them every time you want to use them. Rather than do that, I figured we could just map the object into variable space and let the user then reference them directly.

Hope this ticks most the boxes, if I'm feeling energetic over the weekend I might try do the localisation and testing.

@TomBonnerAtTFL
Copy link
Contributor Author

Hi @bishal-pdMSFT, is there anything else you'd like me to do to progress this PR?

@bishal-pdMSFT
Copy link
Contributor

@TomBonnerAtTFL the changes you have done is to v2 version of task. while that is good, please make same changes in v3 task as well - AzureResourceManagerTemplateDeploymentV3

@bishal-pdMSFT
Copy link
Contributor

Overall the changes look good to me. Please take a look at couple of comments.

@TomBonnerAtTFL
Copy link
Contributor Author

Hi @bishal-pdMSFT, I have made your recommended changes. My apologies for not seeing the AzureResourceManagerTemplateDeploymentV3 task, I have added my changes to that as well now.

@bishal-pdMSFT
Copy link
Contributor

@TomBonnerAtTFL Looks good to me. Left one comment about logging. After that, I can go ahead and merge the PR.

@TomBonnerAtTFL
Copy link
Contributor Author

@bishal-pdMSFT I've added the console lines to both task's output. Good spot.

@TomBonnerAtTFL
Copy link
Contributor Author

Hi @bishal-pdMSFT I have hopefully solved all your comments and it is good to merge if you see fit.

@bishal-pdMSFT bishal-pdMSFT merged commit 1173324 into microsoft:master Nov 23, 2020
@bishal-pdMSFT
Copy link
Contributor

@TomBonnerAtTFL merged 😀. Thank you so much for your contribution and making us better 🎉

@TomBonnerAtTFL
Copy link
Contributor Author

TomBonnerAtTFL commented Nov 23, 2020

I was worried my change had broke your build! But it seems that there is a bug in your CI.

In step "Remove CodeSignSummary file from package" you are seeing the following error, but it does not fail the build process.

image

This means the publish eventually fails due to the extra file

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.

None yet

2 participants