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

adding new version for SignalR: includes MSI support #17789

Merged
merged 1 commit into from
Jan 28, 2021
Merged

adding new version for SignalR: includes MSI support #17789

merged 1 commit into from
Jan 28, 2021

Conversation

juniwang
Copy link
Contributor

@juniwang juniwang commented Jan 6, 2021

No description provided.

@juniwang
Copy link
Contributor Author

Can one of the admins review this PR? thanks

markcowl
markcowl requested changes Jan 12, 2021
View reviewed changes
Copy link
Member

@markcowl markcowl left a comment

Choose a reason for hiding this comment

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

Please add in a link to the swagger PR corresponding to these changes.

It looks like the API removes the 'HostnamePrefix' - this is a breaking change from the previous, stavble api. This requires either mitigation, or a signoff from the braking change review board.

...r/Microsoft.Azure.Management.SignalR/src/Generated/Models/SignalRCreateOrUpdateProperties.cs Show resolved Hide resolved
...r/Microsoft.Azure.Management.SignalR/src/Generated/Models/SignalRCreateOrUpdateProperties.cs Show resolved Hide resolved
sdk/signalr/Microsoft.Azure.Management.SignalR/src/Generated/Models/SignalRResource.cs Show resolved Hide resolved
@markcowl markcowl closed this Jan 12, 2021
@markcowl markcowl reopened this Jan 12, 2021
@juniwang juniwang removed their assignment Jan 15, 2021
markcowl
markcowl requested changes Jan 25, 2021
View reviewed changes
Copy link
Member

@markcowl markcowl left a comment

Choose a reason for hiding this comment

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

If this is a breaking change approved by the azure breaking change board, then that is fine.

To verify, please link to the swagger PR corresponding to this change (it should be marked as an approved breaking change). If this hasn't been through the breaking change process, we will need to loop in the azure breaking change folks.

@markcowl
Copy link
Member

@juniwang Please link to the swagger PR so we can verify the change and the breaking change approval.

@juniwang
Copy link
Contributor Author

Hi Mark, here is the PR for swagger: Azure/azure-rest-api-specs#12101. HostNamePrefix was removed starting from 2020-07-01-preview and changed to readOnly in previous stable version 2020-05-01. If the deletion is unacceptable(we strongly want to delete it, this property has no good but confusing our customers), we can add it back(and I guess another new api-version is needed) and put some warnings on it

@markcowl markcowl merged commit 8f39b46 into Azure:master Jan 28, 2021
@juniwang
Copy link
Contributor Author

Thanks a lot

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@markcowl markcowl markcowl approved these changes

@allenjzhang allenjzhang Awaiting requested review from allenjzhang

@bquantump bquantump Awaiting requested review from bquantump

@m-nash m-nash Awaiting requested review from m-nash

@nisha-bhatia nisha-bhatia Awaiting requested review from nisha-bhatia

@YalinLi0312 YalinLi0312 Awaiting requested review from YalinLi0312

Assignees

@markcowl markcowl

@juniwang juniwang

Labels
Do Not Merge
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@juniwang @markcowl