Skip to content

Adding WebPubSubService provisioning#43027

Merged
JoshLove-msft merged 6 commits intomainfrom
lianwei/wps
May 11, 2024
Merged

Adding WebPubSubService provisioning#43027
JoshLove-msft merged 6 commits intomainfrom
lianwei/wps

Conversation

@vicancy
Copy link
Copy Markdown
Member

@vicancy vicancy commented Mar 27, 2024

Contributing to the Azure SDK

Please see our CONTRIBUTING.md if you are not familiar with contributing to this repository or have questions.

For specific information about pull request etiquette and best practices, see this section.

Adding WebPubSubService provisioning for microsoft/aspire#2495

@vicancy vicancy changed the title Adding WebPubSubService Adding WebPubSubService provisioning Mar 27, 2024
@jsquire jsquire requested review from JoshLove-msft and m-nash March 27, 2024 14:21
@jsquire jsquire requested a review from tg-msft March 27, 2024 14:21
@pallavit
Copy link
Copy Markdown
Contributor

@jsquire do we want to add this to the codeowners list too? I think it would be valuable but I usually see you gatekeep so wanted to check with you.

@jsquire
Copy link
Copy Markdown
Member

jsquire commented Mar 27, 2024

@jsquire do we want to add this to the codeowners list too? I think it would be valuable but I usually see you gatekeep so wanted to check with you.

Yeah, it's past time that @JoshLove-msft and @tg-msft were added as owners for provisioning. I'll put out a PR for this tomorrow if Josh doesn't beat me to it.

@vicancy
Copy link
Copy Markdown
Member Author

vicancy commented Apr 1, 2024

Hi @JoshLove-msft, could you provide guidance on running and testing the test? The CI throws with System.InvalidOperationException : WebPubSubData does not implement IPersistableModel however WebPubSubData does implement IPersistableModel, do you have any hints how to fix the test? Do I need to prepare something for the test?

https://github.com/Azure/azure-sdk-for-net/blob/47a7099bd35293a864a6bdcc6b7640bbad3fdbda/sdk/webpubsub/Azure.ResourceManager.WebPubSub/api/Azure.ResourceManager.WebPubSub.netstandard2.0.cs#L20C211-L20C228

@JoshLove-msft
Copy link
Copy Markdown
Member

Hi @JoshLove-msft, could you provide guidance on running and testing the test? The CI throws with System.InvalidOperationException : WebPubSubData does not implement IPersistableModel however WebPubSubData does implement IPersistableModel, do you have any hints how to fix the test? Do I need to prepare something for the test?

https://github.com/Azure/azure-sdk-for-net/blob/47a7099bd35293a864a6bdcc6b7640bbad3fdbda/sdk/webpubsub/Azure.ResourceManager.WebPubSub/api/Azure.ResourceManager.WebPubSub.netstandard2.0.cs#L20C211-L20C228

Yes, we would need to generate the bicep serialization in the Azure.ResourceManager.WebPubSub library. Here is an example https://github.com/Azure/azure-sdk-for-net/blob/main/sdk/servicebus/Azure.ResourceManager.ServiceBus/src/autorest.md showing the property that needs to be set:

enable-bicep-serialization: true

Once this property is set, you would need to regenerate that library, and then we would need to ship a new GA version with the bicep serialization.

@azure-sdk
Copy link
Copy Markdown
Collaborator

API change check

APIView has identified API level changes in this PR and created following API reviews.

Azure.Provisioning

@vicancy
Copy link
Copy Markdown
Member Author

vicancy commented Apr 28, 2024

Looks like CI fails on unrelated tests
image
Hi @JoshLove-msft could you help review the PR?

Comment thread sdk/provisioning/Azure.Provisioning.WebPubSub/README.md Outdated
Comment thread sdk/provisioning/Azure.Provisioning.WebPubSub/src/WebPubSubHub.cs Outdated
Comment thread sdk/provisioning/Azure.Provisioning.WebPubSub/src/WebPubSubService.cs Outdated
@JoshLove-msft JoshLove-msft merged commit 14f1ccd into main May 11, 2024
@JoshLove-msft JoshLove-msft deleted the lianwei/wps branch May 11, 2024 17:58
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.

5 participants