botservice cli extension #155

swagatmishra2007 · 2018-05-01T20:50:51Z

this changelist contains the following changes -

Bot service python sdk generated from swagger
Botservice cli function code.

azuresdkci · 2018-05-01T20:51:08Z

If this PR is for a new extension or change to an existing extension, use the following to try out the changes in this PR:

docker run -it microsoft/azure-cli:latest
export EXT=<NAME>
pip install --upgrade --target ~/.azure/cliextensions/$EXT "git+https://github.com/swagatmishra2007/azure-cli-extensions.git@master#subdirectory=src/$EXT&egg=$EXT"

derekbekoe · 2018-05-01T21:09:58Z

src/botservice/azext_bot/.vscode/tags

@@ -0,0 +1,478 @@
+!_TAG_FILE_FORMAT	2	/extended format; --format=1 will not append ;" to lines/


Does this file need to be checked-in to the repo?
What's it for?

no it doesn't . i will add my .vscode folder to gitignore so that this doesnt get in by mistake

derekbekoe · 2018-05-01T21:10:22Z

src/botservice/azext_bot/__init__.py

+
+from azure.cli.core import AzCommandsLoader
+from azext_bot._help import helps #pylint: disable=unused-import
+import pdb


Looks like a debug import that should be removed.

derekbekoe · 2018-05-01T21:18:12Z

src/botservice/azext_bot/_help.py

+    examples:
+        - name: Update description on a bot
+          text: |-
+            az bot update -n botname -g resource_group_name --set properties.description="some description"


Prefer MyResourceGroup over resource_group_name

Same elsewhere.

derekbekoe · 2018-05-01T21:21:37Z

src/botservice/azext_bot/custom.py

+
+    if not msa_app_id:
+        msa_app_id, password = provisionConvergedApp(resource_name)
+        print('obtained msa app id and password. Provisioning bot now.')


Use logger.warning instead of print.

derekbekoe · 2018-05-01T21:22:48Z

src/botservice/azext_bot/custom.py

+from azext_bot.botservice import AzureBotService
+from azext_bot.botservice.models import Bot, BotProperties,sku, BotChannel
+from azure.cli.command_modules.appservice.custom import enable_zip_deploy, config_source_control, get_app_settings, _get_site_credential, _get_scm_url
+from azure.cli.command_modules.resource.custom import deploy_arm_template


Depending on other command modules like this is potentially brittle.
The command modules you import have no idea you're using them in this way so this could break the extension in the future.

It's okay to leave it as is since you mentioned you plan to move in to the CLI so it will be less of an issue then but still not recommended.

that is true. longterm, should i talk to the other cli module teams to make sure that they continue to support these functions?

In general, your extension/module should not rely on other modules so you shouldn't have these imports at all. You can either copy the functionality into your module or we could move them into the core so that a common implementation is available.

either of the 2 funcitonalities are acceptable to me. i am thinking of making the changes in another PR though since i dont want to break anything this close to build conference. Is it ok to get this checked in and onto the index.json and then taking the dependency off (by copying the code over)?

derekbekoe · 2018-05-01T21:30:35Z

src/botservice/azext_bot/custom.py

+    deploy_result = deploy_arm_template(
+        cmd = cmd,
+        resource_group_name = resource_group_name,
+        template_file = '{0}\\{1}'.format(dir_path, template_name),


Will this work on Linux/macOS?

changed to use os.path.join instead of string concatenation. That will make it os independent.

derekbekoe · 2018-05-01T21:31:21Z

src/botservice/azext_bot/custom.py

+
+    #since there is no git url it's definitely publish from local
+    if not code_dir:
+        import os


You can really just import os at the very top of the file instead of importing it so many times.
Same with other standard python imports such as shutil

good catch! thanks. .updated the code to move os, shutil and json imports to the top of the file

derekbekoe · 2018-05-01T21:33:37Z

src/botservice/setup.py

+    HISTORY = f.read()
+
+setup(
+    name='azurebotextension',


Change name of extension.
e.g. to just botservice since that's the name of the directory in this PR.
Then user would install with az extension add -n botservice

derekbekoe · 2018-05-01T22:25:15Z

@swagatmishra2007 Thanks for the changes. Please take a look at the CI failure for style.

derekbekoe · 2018-05-02T22:50:33Z

@tjprescott Does it look good to you?

…oad bug

tjprescott

LGTM.

Migrating containerapp-compose into containerapp

botservice cli extension

0237759

derekbekoe suggested changes May 1, 2018

View reviewed changes

address comments

d875c33

Swagat Mishra and others added 5 commits May 2, 2018 13:17

fixed flake8 errors

ca6793c

update code owners for botservice

5ff3af7

add license headers to files

5246f8a

Merge branch 'master' into master

72f72ac

Merge branch 'master' into master

a94b649

derekbekoe approved these changes May 2, 2018

View reviewed changes

update help files for download and publish

7050c2d

Swagat Mishra and others added 8 commits May 2, 2018 16:13

add argument grouping for bot create

4a79daa

latest python sdk

6cdeb99

add exception handling. correct params. update facebook

c45d0ec

fix ci errors

2de5764

update index.json pointing to the botservice package

ec1bd23

Merge branch 'master' into master

65a4e86

fix ci index errors

8e05db9

remove store_true and convert to three state flag. also fix bot downl…

cf5b0ef

…oad bug

derekbekoe approved these changes May 3, 2018

View reviewed changes

remove unnecessary skype and msteams params. fix _ in a param

3f391ef

tjprescott approved these changes May 3, 2018

View reviewed changes

tjprescott merged commit 3b810b4 into Azure:master May 3, 2018

runefa pushed a commit to runefa/azure-cli-extensions that referenced this pull request Sep 23, 2022

Merge pull request Azure#155 from smurawski/compose-migration

56c2777

Migrating containerapp-compose into containerapp

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

botservice cli extension #155

botservice cli extension #155

swagatmishra2007 commented May 1, 2018

azuresdkci commented May 1, 2018

derekbekoe May 1, 2018

swagatmishra2007 May 1, 2018

derekbekoe May 1, 2018

derekbekoe May 1, 2018

derekbekoe May 1, 2018

derekbekoe May 1, 2018

swagatmishra2007 May 1, 2018

derekbekoe May 1, 2018

swagatmishra2007 May 1, 2018

tjprescott May 2, 2018

swagatmishra2007 May 2, 2018

derekbekoe May 1, 2018

swagatmishra2007 May 1, 2018

derekbekoe May 1, 2018

swagatmishra2007 May 1, 2018

derekbekoe May 1, 2018

derekbekoe commented May 1, 2018

derekbekoe commented May 2, 2018

tjprescott left a comment

		@@ -0,0 +1,478 @@
		!_TAG_FILE_FORMAT 2 /extended format; --format=1 will not append ;" to lines/

botservice cli extension #155

botservice cli extension #155

Conversation

swagatmishra2007 commented May 1, 2018

azuresdkci commented May 1, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

derekbekoe commented May 1, 2018

derekbekoe commented May 2, 2018

tjprescott left a comment

Choose a reason for hiding this comment