BotService: Bugfixing, code refactoring, reorganization and UX revamping

[carlosscastro]

The code grew organically and it was time to do some refactors to improve maintainability and provide a more robust experience. We observe very high usage of the bot service CLI, so we are going to invest in this strategic tool moving forward. This Pr contains:

• Refactor of large methods into classes with single responsibilities. Modular design allows us to add unit tests in the upcoming releases.
• Test coverage: Adding end to end tests to now cover for most bot management scenarios. Unit tests to come in next release, but still a huge improvement in automated coverage.
• User Experience: Adding an extremely easy-to-debug bot management experience.

Again, note the core of the code remains unchanged, but reorganized. We did simplifications and bug fixes from bugs that 1) users reported in this repo and other repos or stack overflow and 2) bugs that we found in our new tests.

---

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

• The PR has modified HISTORY.rst describing any customer-facing, functional changes. Note that this does not include changes only to help content. (see Modifying change log).

• I adhere to the Command Guidelines.

[tjprescott]

Does this PR fix any of the open in issues in this repo?
https://github.com/Azure/azure-cli/issues?q=is%3Aopen+is%3Aissue+label%3ABotService

[tjprescott]

Why is this not an enum in your SDK (which is another way of saying, why isn't this specified in Swagger)?

[carlosscastro]

Because this is not information that flows in the swagger, but higher level information. As you may know, we offer the basic CRUD operations for bot such as Create / Update / delete / Show but also some other operations that are outside of the basic crud such as Download and Publish for downloading and publishing bots. The languages are a construct of download and publish, but crud operations don't need to know the language generally.

The other scenario where languages are known is when we create a website for the bot (bot.kind == webapp). In which case we use the language to decide which bot template to publish to the website, but again it does not interact with our arm api.

Hope that makes sense, I'm happy to elaborate further.

[tjprescott]

@yugangw-msft for auth-related code.

[carlosscastro]

FYI, This code was already in the core CLI, I just refactored it into an external class because it was mingled with the bot creation code.

The purpose of this is, to create a bot we need a Microsoft Application Id. Users can either a) provide an id or b) ask us to provision one for them, case in which we authenticate to request permissions to provision an application and we create it and use it for the bot.

But again this was in custom.py before this PR so you already reviewed it likely.

Happy to provide more details!

[carlosscastro]

Hello @tjprescott, thanks for your review.

A large part of our user base uses botbuilder-tools to file issues so we are fixing a lot of bugs from there. I followed up on all bugs on this repo. They are mostly questions though, but I'll re-check one by one to see which are fixed. But overall we are fixing tens of bugs we found and adding a much better user experience. Since we've been using the extension, the bits published now in the core CLI for botservice are not in a great shape, so this will be a huge improvement for users.

Question regarding the build failure on this PR. We are getting the error below, but I don't see any error in my local computer. How can I reproduce this command locally to work faster? Also any hints of what the error could be here?

1.0.2 wcwidth-0.1.7 websocket-client-0.54.0 whoosh-2.7.4 xmltodict-0.11.0 yarl-1.2.6
Initializing linter with command table and help files...
Traceback (most recent call last):
  File "/home/travis/virtualenv/python3.6.3/bin/azdev", line 11, in <module>
    load_entry_point('azure-cli-dev-tools', 'console_scripts', 'azdev')()
  File "/home/travis/build/Azure/azure-cli/tools/automation/__main__.py", line 25, in main
    args.func(args)
  File "/home/travis/build/Azure/azure-cli/tools/automation/cli_linter/__init__.py", line 38, in main
    create_invoker_and_load_cmds_and_args(az_cli)
  File "/home/travis/virtualenv/python3.6.3/lib/python3.6/site-packages/azure/cli/core/file_util.py", line 57, in create_invoker_and_load_cmds_and_args
    invoker.commands_loader.load_arguments(command)
  File "/home/travis/virtualenv/python3.6.3/lib/python3.6/site-packages/azure/cli/core/__init__.py", line 253, in load_arguments
    self.command_table[command].load_arguments()  # this loads the arguments via reflection
  File "/home/travis/virtualenv/python3.6.3/lib/python3.6/site-packages/azure/cli/core/commands/__init__.py", line 141, in load_arguments
    super(AzCliCommand, self).load_arguments()
  File "/home/travis/virtualenv/python3.6.3/lib/python3.6/site-packages/knack/commands.py", line 76, in load_arguments
    cmd_args = self.arguments_loader()
  File "/home/travis/virtualenv/python3.6.3/lib/python3.6/site-packages/azure/cli/core/__init__.py", line 444, in default_arguments_loader
    op = handler or self.get_op_handler(operation)
  File "/home/travis/virtualenv/python3.6.3/lib/python3.6/site-packages/azure/cli/core/__init__.py", line 489, in get_op_handler
    op = import_module(mod_to_import)
  File "/home/travis/virtualenv/python3.6.3/lib/python3.6/importlib/__init__.py", line 126, in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
  File "<frozen importlib._bootstrap>", line 994, in _gcd_import
  File "<frozen importlib._bootstrap>", line 971, in _find_and_load
  File "<frozen importlib._bootstrap>", line 955, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 665, in _load_unlocked
  File "<frozen importlib._bootstrap_external>", line 678, in exec_module
  File "<frozen importlib._bootstrap>", line 219, in _call_with_frames_removed
  File "/home/travis/virtualenv/python3.6.3/lib/python3.6/site-packages/azure/cli/command_modules/botservice/bot_operations.py", line 10, in <module>
    from azure.cli.command_modules.botservice.auth import converged_app
ModuleNotFoundError: No module named 'azure.cli.command_modules.botservice.auth'

[stevengum]

@tjprescott do you have any feedback on this import error?

In the latest build for the 2.7 Automation, the tests are not being imported properly due to the following reason: "Reason: No module named cli.core"

==================
Discover Tests
==================
�[0m
�[0mUnable to import azure.cli.command_modules.botservice.tests.latest.test_authsetting_commands. Reason: No module named cli.core�[0m
�[0mUnable to import azure.cli.command_modules.botservice.tests.latest.test_channel_commands. Reason: No module named cli.core�[0m
�[0mUnable to import azure.cli.command_modules.botservice.tests.latest.test_bot_commands. Reason: No module named cli.core�[0m

The log can be found here: https://api.travis-ci.org/v3/job/460375761/log.txt

[tjprescott]

For this ModuleNotFoundError: No module named 'azure.cli.command_modules.botservice.auth' you likely need to add it to the package list in your setup.py file. Same for the other subdirectories you created in your refactoring.

[stevengum]

For this ModuleNotFoundError: No module named 'azure.cli.command_modules.botservice.auth' you likely need to add it to the package list in your setup.py file. Same for the other subdirectories you created in your refactoring.

I managed to get this in with this commit, so that error should go away. There was a similar error popping up for tests/latest/tools/, but I was hesitant to add the following to the setup.py file:

'azure.cli.command_modules.botservice.tests',
'azure.cli.command_modules.botservice.tests.latest',
'azure.cli.command_modules.botservice.tests.latest.tools'

My hesitance stems from the fact that we don't want to expose the testing apparatuses to users to use; is there a workaround that we can use for the test folders? perhaps just from .tools.directline_client import DirectLineClient?

[tjprescott]

In the test folders for the resource module, we have to reference certain JSON files. We do something like:

curr_dir = os.path.dirname(os.path.realpath(__file__))
os.path.join(curr_dir, 'foo.json').replace('\\', '\\\\')

That may or may not help with your scenario.

[carlosscastro]

Thanks for your help with the build @tjprescott. We found some issues in python 2.7 when the folder structure was more nested so we just made it flattened as before.