[Spring Cloud] Add buildpack-binding command for Enterprise tier. #4302
Conversation
* Add create, show, set, delete and list for buildpack-binding. * Add enable app insights when create Enterprise tier. Signed-off-by: Pan Li <[email protected]>
|
The test part is under preparing, will be soon. |
|
Spring-Cloud |
Signed-off-by: Pan Li <[email protected]>
Signed-off-by: Pan Li <[email protected]>
Signed-off-by: Pan Li <[email protected]>
Signed-off-by: Pan Li <[email protected]>
Signed-off-by: Pan Li <[email protected]>
Signed-off-by: Pan Li <[email protected]>
| namespace.builder_name, | ||
| namespace.name) | ||
| if binding_resource is not None: | ||
| raise CLIError('buildpack Binding {} in builder {} already exists ' |
There was a problem hiding this comment.
Could you please use a specific error type instead of CLIError ?
| 'You can visit %s/#resource%s/overview to view your ' | ||
| 'Application Insights component', appinsights.name, portal_url, appinsights.id) | ||
|
|
||
| return appinsights |
There was a problem hiding this comment.
Please add newline at end of file.
| 'Format "key[=value]".', | ||
| nargs='*', | ||
| validator=validate_buildpack_binding_secrets) | ||
| c.argument('name', help='Name for buildpack binding.', validator=validate_buildpack_binding_not_exist) |
There was a problem hiding this comment.
| c.argument('name', help='Name for buildpack binding.', validator=validate_buildpack_binding_not_exist) | |
| c.argument('name', name_type, help='Name for buildpack binding.', validator=validate_buildpack_binding_not_exist) |
| 'Format "key[=value]".', | ||
| nargs='*', | ||
| validator=validate_buildpack_binding_secrets) | ||
| c.argument('name', help='Name for buildpack binding.', validator=validate_buildpack_binding_exist) |
There was a problem hiding this comment.
| c.argument('name', help='Name for buildpack binding.', validator=validate_buildpack_binding_exist) | |
| c.argument('name', name_type, help='Name for buildpack binding.', validator=validate_buildpack_binding_exist) |
| for scope in ['spring-cloud build-service builder buildpack-binding show', | ||
| 'spring-cloud build-service builder buildpack-binding delete']: | ||
| with self.argument_context(scope) as c: | ||
| c.argument('name', help='Name for buildpack binding.', validator=validate_buildpack_binding_exist) |
There was a problem hiding this comment.
| c.argument('name', help='Name for buildpack binding.', validator=validate_buildpack_binding_exist) | |
| c.argument('name', name_type, help='Name for buildpack binding.', validator=validate_buildpack_binding_exist) |
wangzelin007
changed the title
| c.argument('app', app_name_type, help='Name of app.', validator=validate_app_name) | ||
|
|
||
| with self.argument_context('spring-cloud service-registry unbind') as c: | ||
| c.argument('app', app_name_type, help='Name of app.', validator=validate_app_name) |
There was a problem hiding this comment.
Duplicate with line 437, please delete line 429-433
|
|
||
| for scope in ['spring-cloud build-service builder buildpack-binding show', | ||
| 'spring-cloud build-service builder buildpack-binding delete']: | ||
| with self.argument_context(scope) as c: |
There was a problem hiding this comment.
| with self.argument_context(scope) as c: | |
| for scope in ['show', 'delete']: | |
| with self.argument_context('spring-cloud build-service builder buildpack-binding {}'.format(scope)) as c: |
| c.argument('service', service_name_type, validator=only_support_enterprise) | ||
|
|
||
|
|
||
| for scope in ['spring-cloud build-service builder buildpack-binding set']: |
There was a problem hiding this comment.
It's better to merge spring-cloud build-service builder buildpack-binding create with set except the argument name.
Otherwise we don't need to use scope here.
There was a problem hiding this comment.
One question, is this the convention to merge such kinds of statements? If so, I can align this.
As a beginner to CLI, I intended to write parms like this, because it is clean when I look into one command for parameters. I can have a full picture of all parameters of one command.
Compare to merging, I may need to combine these in my mind for full picture.
Could you please help to enlighten me what is the best practice here?
There was a problem hiding this comment.
Hi Li,
We highly recommend merging, because after merging, the similarities and differences can be more clearly distinguished.
If you modify it later，it is not easy to miss anything after merge.
There was a problem hiding this comment.
sure, updated.
Signed-off-by: Pan Li <[email protected]>
Signed-off-by: Pan Li <[email protected]>
|
@wangzelin007 test added. |
Signed-off-by: Pan Li <[email protected]>
Signed-off-by: Pan Li <[email protected]>
| helps['spring-cloud build-service builder buildpack-binding list'] = """ | ||
| type: command | ||
| short-summary: (Support Enterprise Tier Only) List all buildpack binding in a builder. The secrets will be masked. | ||
| examples: | ||
| - name: Show a buildpack binding. | ||
| text: az spring-cloud build-service builder buildpack-binding list --builder-name first-builder --service MyCluster --resource-group MyResourceGroup | ||
| """ |
There was a problem hiding this comment.
name: Show a buildpack binding.
Why is the description here Show rather than list?
There was a problem hiding this comment.
Good catch, thank you!
|
|
||
| from azure.cli.core.commands.validators import validate_tag | ||
| from azure.core.exceptions import ResourceNotFoundError | ||
| from azure.cli.core.util import CLIError |
There was a problem hiding this comment.
from azure.cli.core.util import CLIError
Please remove the useless import
|
@Incarnation-p-lee Could you please fix the CI issues? |
Signed-off-by: Pan Li <[email protected]>
|
@zhoxing-ms Sure, will be soon. |
Signed-off-by: Pan Li <[email protected]>
| g.custom_command('set', 'create_or_update_buildpack_binding') | ||
| g.custom_show_command('show', 'buildpack_binding_show') | ||
| g.custom_command('list', 'buildpack_binding_list') | ||
| g.custom_command('delete', 'buildpack_binding_delete') |
There was a problem hiding this comment.
Do we need consider adding confirmation=True for the delete command?
There was a problem hiding this comment.
make sense, let me update it.
Signed-off-by: Pan Li <[email protected]>
Signed-off-by: Pan Li <[email protected]>
Signed-off-by: Pan Li <[email protected]>
Signed-off-by: Pan Li <[email protected]>
Signed-off-by: Pan Li <[email protected]>
| ]) | ||
|
|
||
| results = self.cmd('spring-cloud build-service builder buildpack-binding list -g {rg} -s {serviceName}').get_output_in_json() | ||
| self.assertEqual(2, len(results)) |
There was a problem hiding this comment.
Please add newline at end of file.
Signed-off-by: Pan Li <[email protected]>
Incarnation-p-lee
deleted the
panli/add-buildpack-binding-command-for-enterprise
branch
Signed-off-by: Pan Li [email protected]
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.