EventGrid CLI Extension: 2020-04-01-preview bug fixes #1538
Conversation
|
add to S168 |
|
|
||
| print('====> params') |
There was a problem hiding this comment.
print is not recommended in azure cli. Please use logger.
There was a problem hiding this comment.
Thanks. was using for debugging and forgot to remove.
| c.argument('topic_name', arg_type=topic_name_type, id_part=None) | ||
|
|
||
| with self.argument_context('eventgrid topic private-endpoint-connection approve') as c: | ||
| c.argument('topic_name', arg_type=topic_name_type, id_part=None) |
There was a problem hiding this comment.
Do we support connection id to specify a private end point connection?
The same for delete/show/approve/reject commands?
There was a problem hiding this comment.
No (at least not currently).
There was a problem hiding this comment.
Because there is no functionality difference with connection-id support, I will recommend you to add this support. In CLI other modules, for example, storage, keyvault, mysql, cosmosdb..., we all have the same interface as document shown.
There was a problem hiding this comment.
OK. thanks for the recommendation. We will not fix this as part of this PR and will evaluate it for future changes. Please feel free to open separate issue for that and Azure event grid service team will evaluate if we need to fix this and when. This should not block this PR. thx
There was a problem hiding this comment.
Could you please try to follow this guideline for private link connection to keep consistent with other service? https://github.com/Azure/azure-cli/blob/dev/doc/private_endpoint_connection_command_guideline.md#command-guideline
====> We are following these guidlines to the best we can. our requirements is a little bit different than keyvalult as we have multiple resources to handle (topic and domain and others).
however, we changed the approval-description and rejection-description to only description to match this guidelines. all other operations (show/list/delete/approve/reject) are matching
|
@Juliehzl All comments addressed. thanks. |
This checklist is used to make sure that common guidelines for a pull request are followed.
General Guidelines
azdev style <YOUR_EXT>locally? (pip install azdevrequired)python scripts/ci/test_index.py -qlocally?For new extensions:
About Extension Publish
There is a pipeline to automatically build, upload and publish extension wheels.
Once your PR is merged into master branch, a new PR will be created to update
src/index.jsonautomatically.The precondition is to put your code inside this repo and upgrade the version in the PR but do not modify
src/index.json.