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.

Sign up for GitHub

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

Jump to bottom

Update Azure Cloud Shell for their new URI format #16247

Merged
merged 9 commits into from
Nov 8, 2023
Merged

Update Azure Cloud Shell for their new URI format #16247

merged 9 commits into from
Nov 8, 2023

Conversation

PankajBhojwani
Copy link
Contributor

@PankajBhojwani PankajBhojwani commented Oct 31, 2023 •
edited by DHowett
Loading

The Azure cloud shell team made some API changes that required us to format our requests a little differently. This PR makes those changes (more info in the comments in the code)

Closes #16098

PankajBhojwani
PankajBhojwani commented Oct 31, 2023
View reviewed changes
src/cascadia/TerminalConnection/AzureConnection.cpp
@@ -398,6 +398,7 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation

switch (bufferType)
{
case WINHTTP_WEB_SOCKET_BINARY_MESSAGE_BUFFER_TYPE:
Copy link
Contributor Author

@PankajBhojwani PankajBhojwani Oct 31, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

The bufferType they return seems to have changed to BINARY_MESSAGE_BUFFER_TYPE, just adding that case here seems to work fine, not sure if this has other implications though

@github-actions GitHub Actions

This comment has been minimized.

Sign in to view
@microsoft-github-policy-service microsoft-github-policy-service bot added Issue-Bug It either shouldn't be doing this or needs an investigation. Area-AzureShell Workitems pertaining to the Azure Cloud Shell connection. Product-Terminal The new Windows Terminal. labels Oct 31, 2023
DHowett
DHowett approved these changes Oct 31, 2023
View reviewed changes
Copy link
Member

@DHowett DHowett left a comment

Choose a reason for hiding this comment

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

unconventional, but looks like it's operating within spec. thanks for driving this down!

lhecker
lhecker requested changes Nov 1, 2023
View reviewed changes
Copy link
Member

@lhecker lhecker left a comment

Choose a reason for hiding this comment

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

(Quick question regarding //. I think it might be important to check it out first.)

@microsoft-github-policy-service microsoft-github-policy-service bot added Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something and removed Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something labels Nov 1, 2023
@microsoft-github-policy-service microsoft-github-policy-service bot added the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Nov 7, 2023
@microsoft-github-policy-service microsoft-github-policy-service bot removed the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Nov 7, 2023
@DHowett DHowett changed the title Fix being unable to connect to Azure Cloud Shell Update Azure Cloud Shell for their new URI format Nov 8, 2023
@DHowett DHowett merged commit 5a9f352 into main Nov 8, 2023
15 of 17 checks passed
@DHowett DHowett deleted the dev/pabhoj/cloud_shell_fix branch November 8, 2023 17:12
radu-cernatescu pushed a commit to radu-cernatescu/terminal that referenced this pull request Nov 8, 2023
@PankajBhojwani @radu-cernatescu
Update Azure Cloud Shell for their new URI format (microsoft#16247)
96ab360 
The Azure cloud shell team made some API changes that required us to
format our requests a little differently. This PR makes those changes
(more info in the comments in the code)

Closes microsoft#16098
DHowett pushed a commit that referenced this pull request Nov 13, 2023
@PankajBhojwani @DHowett
Update Azure Cloud Shell for their new URI format (#16247)
883b776 
The Azure cloud shell team made some API changes that required us to
format our requests a little differently. This PR makes those changes
(more info in the comments in the code)

Closes #16098

(cherry picked from commit 5a9f352)
Service-Card-Id: 90985892
Service-Version: 1.18
DHowett pushed a commit that referenced this pull request Nov 13, 2023
@PankajBhojwani @DHowett
Update Azure Cloud Shell for their new URI format (#16247)
a7409ea 
The Azure cloud shell team made some API changes that required us to
format our requests a little differently. This PR makes those changes
(more info in the comments in the code)

Closes #16098

(cherry picked from commit 5a9f352)
Service-Card-Id: 90985893
Service-Version: 1.19
@adonis-jimenez adonis-jimenez mentioned this pull request Nov 18, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zadjii-msft zadjii-msft zadjii-msft approved these changes

@lhecker lhecker lhecker approved these changes

@DHowett DHowett DHowett approved these changes

Assignees
No one assigned
Labels
Area-AzureShell Workitems pertaining to the Azure Cloud Shell connection. Issue-Bug It either shouldn't be doing this or needs an investigation. Product-Terminal The new Windows Terminal.
Projects
1.18 Servicing Pipeline
Status: Shipped
1.19 Servicing Pipeline
Status: Shipped
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Cannot Connect to Cloud Shell anymore
4 participants
@PankajBhojwani @DHowett @lhecker @zadjii-msft