-
Notifications
You must be signed in to change notification settings - Fork 48
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
update cloudshell runner to use azure cloudshell api #124
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.
just a few small things i have noticed, but other than that looks good!
@@ -35,287 +29,89 @@ export async function getUserSettings(accessToken: string, armEndpoint: string): | |||
}); | |||
|
|||
if (response.statusCode < 200 || response.statusCode > 299) { | |||
// if (response.body && response.body.error && response.body.error.message) { |
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.
so why it's not handled in any way?
src/cloudConsoleLauncher.ts
Outdated
resourceGroup: storageAccountSettings[3], | ||
storageAccountName: storageAccountSettings[7], | ||
fileShareName: storageProfile.fileShareName, | ||
storageAccountKey, |
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.
coma at the end is not necessary
terminal.show(); | ||
TelemetryClient.sendEvent('cloudshell', { 'status': CloudShellStatus.Succeeded }); | ||
|
||
terminal.sendText('ansible-playbook ' + remotePlaybookPath); |
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.
again, i think this should be updated accordingly to match other pr
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.
merged
Summary
update cloudshell runner to use azure cloudshell api. previously there's no api to create cloudshell connection, now cloudshell team release it. update cloudshell runner to use api, which has status.
Issue Type