Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

azdev scaffolding commands and setup tweaks #85

Merged
merged 16 commits into from
Jun 7, 2019
Merged

azdev scaffolding commands and setup tweaks #85

merged 16 commits into from
Jun 7, 2019

Conversation

tjprescott
Copy link
Member

@tjprescott tjprescott commented May 21, 2019

Fixes #18. Fixes #63.

@tjprescott tjprescott requested a review from marstr May 21, 2019 15:29
@tjprescott tjprescott requested a review from zikalino June 4, 2019 20:52
@tjprescott
Copy link
Member Author

@zikalino @achandmsft here is the PR for the cli/extension create commands.

@zikalino
Copy link

zikalino commented Jun 4, 2019

@tjprescott thank you! it's pretty large, I will put some comments very soon.
I am actually wondering how we could do more with some additional information from swagger.

@tjprescott
Copy link
Member Author

@zikalino we were planning to more seriously investigate auto-generation of commands from Swagger but decided this has value in its own right and is pretty low-cost.

I will likely add an interactive mode for this command (similar to azdev setup) to better walk newcomers through the process.

@zikalino
Copy link

zikalino commented Jun 5, 2019

@tjprescott I agree, I think we should definitely merge as it definitely creates very good framework that can be extended.

@@ -225,17 +254,3 @@
short-summary: >
List the repositories that will be searched for in-development extensions.
"""

Copy link

Choose a reason for hiding this comment

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

that was something what was removed before, right?

Copy link

@zikalino zikalino left a comment

Choose a reason for hiding this comment

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

PR looks really good. I think it shoudl be merged.
Also very good reference for me on adding new command modules, so i want to use it asap :-)

@tjprescott
Copy link
Member Author

Thanks @zikalino. I have a few final points that will add the right annotations to the CODEOWNERS file and some other little housekeeping items, but I hope to merge and release today or tomorrow.

@zikalino
Copy link

zikalino commented Jun 7, 2019

@tjprescott I forgot to mention, I used your branch and have tested it and it works for me!

@zikalino
Copy link

zikalino commented Jun 7, 2019

I forgot to mention one more thing.
I have generated apimanagement command, and there was one exception, to fix it I had to make change in one of the autogenerated files.

I didn't dig any further, perhaps my azure-cli code was out of sync or something like that...

/workspaces/azure-cli/src/command_modules/azure-cli-apimanagement/azure/cli/command_modules/apimanagement/commands.py

Just at the end of file i removed is_preview=True from following code:

    with self.command_group('apimanagement', is_preview=True):
        pass

The exception was:

The command failed with an unexpected error. Here is the traceback:

unrecognized kwargs: ['is_preview']
Traceback (most recent call last):
  File "/azure-cli/env/lib/python3.6/site-packages/knack/cli.py", line 206, in invoke
    cmd_result = self.invocation.execute(args)
  File "/workspaces/azure-cli/src/azure-cli-core/azure/cli/core/commands/__init__.py", line 439, in execute
    self.commands_loader.load_command_table(args)
  File "/workspaces/azure-cli/src/azure-cli-core/azure/cli/core/__init__.py", line 247, in load_command_table
    _update_command_table_from_modules(args)
  File "/workspaces/azure-cli/src/azure-cli-core/azure/cli/core/__init__.py", line 144, in _update_command_table_from_modules
    module_command_table, module_group_table = _load_module_command_loader(self, args, mod)
  File "/workspaces/azure-cli/src/azure-cli-core/azure/cli/core/commands/__init__.py", line 893, in _load_module_command_loader
    return _load_command_loader(loader, args, mod, 'azure.cli.command_modules.')
  File "/workspaces/azure-cli/src/azure-cli-core/azure/cli/core/commands/__init__.py", line 875, in _load_command_loader
    command_table = command_loader.load_command_table(args)
  File "/workspaces/azure-cli/src/command_modules/azure-cli-apimanagement/azure/cli/command_modules/apimanagement/__init__.py", line 24, in load_command_table
    load_command_table(self, args)
  File "/workspaces/azure-cli/src/command_modules/azure-cli-apimanagement/azure/cli/command_modules/apimanagement/commands.py", line 26, in load_command_table
    with self.command_group('apimanagement', is_preview=True):
  File "/workspaces/azure-cli/src/azure-cli-core/azure/cli/core/__init__.py", line 422, in command_group
    return self._command_group_cls(self, group_name, **kwargs)
  File "/workspaces/azure-cli/src/azure-cli-core/azure/cli/core/commands/__init__.py", line 989, in __init__
    merged_kwargs = self._merge_kwargs(kwargs, base_kwargs=command_loader.module_kwargs)
  File "/workspaces/azure-cli/src/azure-cli-core/azure/cli/core/commands/__init__.py", line 1013, in _merge_kwargs
    return _merge_kwargs(kwargs, base, CLI_COMMAND_KWARGS)
  File "/workspaces/azure-cli/src/azure-cli-core/azure/cli/core/commands/__init__.py", line 964, in _merge_kwargs
    raise TypeError('unrecognized kwargs: {}'.format(unrecognized_kwargs))
TypeError: unrecognized kwargs: ['is_preview']

@tjprescott
Copy link
Member Author

@zikalino in regards to the is_preview kwarg, you need to have the latest bits from the dev branch. The min CLI core version for these commands is 2.0.66.

@tjprescott tjprescott merged commit 69f6cfe into Azure:master Jun 7, 2019
@tjprescott tjprescott deleted the ModuleCreateCommand branch June 7, 2019 22:12
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.

Paths do not "expand user" in interactive setup Add command to generate module or extension template
3 participants