-
Notifications
You must be signed in to change notification settings - Fork 796
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
User Story 90135 #225
User Story 90135 #225
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
@grayzu please kick off the E2E test |
@grayzu @stemaMSFT @lonegunmanb I saw the build error and tested the sample again on my local machine. As you can see from the image, the sample works. I don't know why it doesn't work in the E2E testing. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @TomArcherMsft for the update, some comments here.
} | ||
|
||
# Output the Service Principal and password | ||
output "sp" { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we move these output
blocks to outputs.tf
file?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@lonegunmanb Would it be possible to have the SP.tf and SSH.tf files fully encapsulated so that they stand alone and can be copied between projects more easily?
response_export_values = ["publicKey"] | ||
} | ||
|
||
output "key_data" { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we move this output
block to outputs.tf
file?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@lonegunmanb I'd like to keep the SSH.tf file fully encapsulated so that I can easily copy it to other projects. If I split some of its functionality across multiple files, that task becomes more difficult as I then have to remember to copy the file and update another file.
@lonegunmanb I made all the changes except for the 2 where I left a comment. Thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @TomArcherMsft for the update, LGTM!
User Story 90135