Skip to content

Conversation

@bjqian
Copy link
Contributor

@bjqian bjqian commented Jun 27, 2022

remove hard restriction on sku. Leave this to server side for future Premium_P1, Standard_S2


This checklist is used to make sure that common guidelines for a pull request are followed.

Related command

az webpubsub

General Guidelines

  • Have you run azdev style <YOUR_EXT> locally? (pip install azdev required)
  • Have you run python scripts/ci/test_index.py -q locally?

For new extensions:

About Extension Publish

There is a pipeline to automatically build, upload and publish extension wheels.
Once your pull request is merged into main branch, a new pull request will be created to update src/index.json automatically.
The precondition is to put your code inside this repository and upgrade the version in the pull request but do not modify src/index.json.

remove hard restriction on sku. Leave this to server side for future Premium_P1, Standard_S2
@bjqian bjqian requested a review from zackliu June 27, 2022 03:15
@yonzhan
Copy link
Collaborator

yonzhan commented Jun 27, 2022

webpubsub

@yonzhan yonzhan requested a review from kairu-ms June 27, 2022 03:31
@yonzhan yonzhan added this to the Jun 2022 (2022-07-05) milestone Jun 27, 2022
with self.argument_context('webpubsub create') as c:
c.argument('sku', arg_type=get_enum_type(SKU_TYPE), help='The sku name of the signalr service.')
c.argument('unit_count', help='The number of signalr service unit count', type=int)
c.argument('sku', help='The sku name of the webpubsub service. E.g. Standard_S1')
Copy link
Contributor

Choose a reason for hiding this comment

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

why diable the choice options of sku argument?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We have SKUs that are internal only. Curretly we don't hope customers outside Microsoft to see those SKUs as they are not allowed to create them. But we hope az cli is available for our internal customers. So we need to remove this restriction from client side and leave it to server side.

Copy link
Contributor

Choose a reason for hiding this comment

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

The sku parameter is required for az webpubsub create. Did you mean the customers outside Microsoft are not supposed to run the create command?

Copy link
Member

Choose a reason for hiding this comment

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

I think we can put all sku available for public in help. But as it's a server-side parameter, I think it doesn't make sense to make restrictions in cli side. We can pass sku parameter to RP side for checking.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can put all sku available for public in help. But as it's a server-side parameter, I think it doesn't make sense to make restrictions in cli side. We can pass sku parameter to RP side for checking.

I totally agree

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

@kairu-ms kairu-ms merged commit 3bb6fb5 into Azure:main Jun 29, 2022
@kairu-ms
Copy link
Contributor

@bjqian If you want to release a new version, please create a new PR with extension version updated.

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.

4 participants