Skip to content

Support CLI arguments for cfn init #574

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

Merged
merged 13 commits into from
Oct 2, 2020
91 changes: 80 additions & 11 deletions src/rpdk/core/init.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,12 @@
"""
import logging
import re
from argparse import SUPPRESS
from functools import wraps

from colorama import Fore, Style

from .exceptions import WizardAbortError, WizardValidationError
from .plugin_registry import PLUGIN_CHOICES
from .plugin_registry import get_plugin_choices
Copy link
Contributor

Choose a reason for hiding this comment

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

any reason why we are adding this method in? seems to be the same value as PLUGIN_CHOICES

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For testing we add a dummy plugin on the fly but at that point in time PLUGIN_CHOICES is already a constant that does not includes it, making it a method allows to get the latest installed plugins.

from .project import Project

LOG = logging.getLogger(__name__)
Expand Down Expand Up @@ -42,7 +41,7 @@ def validate_type_name(value):
return value
LOG.debug("'%s' did not match '%s'", value, TYPE_NAME_REGEX)
raise WizardValidationError(
"Please enter a value matching '{}'".format(TYPE_NAME_REGEX)
"Please enter a resource type name matching '{}'".format(TYPE_NAME_REGEX)
)


Expand Down Expand Up @@ -77,7 +76,7 @@ def __call__(self, value):


validate_plugin_choice = ValidatePluginChoice( # pylint: disable=invalid-name
PLUGIN_CHOICES
get_plugin_choices()
)


Expand Down Expand Up @@ -139,14 +138,39 @@ def init(args):

check_for_existing_project(project)

type_name = input_typename()
if args.type_name:
try:
type_name = validate_type_name(args.type_name)
except WizardValidationError as error:
print(Style.BRIGHT, Fore.RED, str(error), Style.RESET_ALL, sep="")
Copy link
Contributor

Choose a reason for hiding this comment

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

can we do it a bit more generic and reusable?

type_name = input_typename()
else:
type_name = input_typename()

if args.language:
language = args.language
LOG.warning("Language plugin '%s' selected non-interactively", language)
language = args.language.lower()
if language not in get_plugin_choices():
print(
Style.BRIGHT,
Fore.RED,
"The plugin for {} is not installed.".format(language),
Copy link
Contributor

@johnttompkins johnttompkins Sep 15, 2020

Choose a reason for hiding this comment

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

Suggested change
"The plugin for {} is not installed.".format(language),
"The plugin for {} cannot be found.".format(language),

Copy link
Contributor

Choose a reason for hiding this comment

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

we probably should not always print colorama otherwise it will be really hard to maintain;
maybe we can use for an abstract exception which all will change color entry

Style.RESET_ALL,
sep="",
)
language = input_language()
else:
language = input_language()

project.init(type_name, language)
project.init(
type_name,
language,
{
"use_docker": args.use_docker,
"namespace": args.namespace,
"codegen_template_path": args.codegen_model,
Copy link
Contributor

Choose a reason for hiding this comment

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

codegen-model is specific to java plugin and not supported by any other ones; will this fail project init if it's provided? if not we should and assert that it's not supported by the plugin;

Copy link
Contributor Author

@chuchocd85 chuchocd85 Sep 16, 2020

Choose a reason for hiding this comment

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

At the moment, each plugin looks for the arguments it needs in project.settings, the fact that additional un-related information is also coming in does not affects the plugin. We sure can do some validation and only allow arguments related to the selected plugin but I don't know if is worth doing that validation or how it will impact the user experience, if the user passes a wrong flag s/he would need to try again (kind of defeating the purpose of being able to use cfn init as just a one-liner). What do you think?

"importpath": args.import_path,
},
)
project.generate()
project.generate_docs()

Expand All @@ -172,7 +196,52 @@ def setup_subparser(subparsers, parents):
parser.set_defaults(command=ignore_abort(init))

parser.add_argument(
"--force", action="store_true", help="Force files to be overwritten."
"-f",
"--force",
action="store_true",
help="Force files to be overwritten.",
)

parser.add_argument(
"-l",
"--language",
help="""Select a language for code generation.
The language plugin needs to be installed.""",
)

parser.add_argument(
"-t",
"--type-name",
help="Select the name of the resource type.",
)

parser.add_argument(
"-d",
"--use-docker",
action="store_true",
help="""Use docker for python platform-independent packaging.
This is highly recommended unless you are experienced
with cross-platform Python packaging.""",
)

parser.add_argument(
"-n",
"--namespace",
Copy link
Contributor

Choose a reason for hiding this comment

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

@jotompki do you think we could use something like a config file to specify all the metadata - ConfigArgParse is one of the options

nargs="?",
const="default",
help="""Select the name of the Java namespace.
Passing the flag without argument select the default namespace.""",
)

parser.add_argument(
"-c",
"--codegen-model",
choices=["default", "guided_aws"],
help="Select a codegen model.",
)

parser.add_argument(
"-p",
"--import-path",
help="Select the go language import path.",
)
# this is mainly for CI, so suppress it to keep it simple
parser.add_argument("--language", help=SUPPRESS)
8 changes: 7 additions & 1 deletion src/rpdk/core/plugin_registry.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,13 @@
for entry_point in pkg_resources.iter_entry_points("rpdk.v1.languages")
}

PLUGIN_CHOICES = sorted(PLUGIN_REGISTRY.keys())

def get_plugin_choices():
plugin_choices = [
entry_point.name
for entry_point in pkg_resources.iter_entry_points("rpdk.v1.languages")
]
Comment on lines +10 to +13
Copy link
Contributor

Choose a reason for hiding this comment

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

I would use set if you dont rely on element position

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Before my changes the list of plugins was already being sorted which converts the set back to a list. I f this is not required anymore I'll be ok with making it a set.

return sorted(plugin_choices)


def load_plugin(language):
Expand Down
110 changes: 87 additions & 23 deletions tests/test_init.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
from pathlib import Path
from unittest.mock import ANY, Mock, PropertyMock, patch

import pytest
Expand All @@ -17,24 +16,19 @@
)
from rpdk.core.project import Project

from .utils import add_dummy_language_plugin, get_args, get_mock_project

PROMPT = "MECVGD"
ERROR = "TUJFEL"


def test_init_method_interactive_language():
def test_init_method_interactive():
type_name = object()
language = object()

args = Mock(spec_set=["force", "language"])
args.force = False
args.language = None

mock_project = Mock(spec=Project)
mock_project.load_settings.side_effect = FileNotFoundError
mock_project.settings_path = ""
mock_project.root = Path(".")
args = get_args(interactive=True)
Copy link
Contributor

Choose a reason for hiding this comment

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

is this even being used in this test other than the settings?

mock_project, patch_project = get_mock_project()

patch_project = patch("rpdk.core.init.Project", return_value=mock_project)
patch_tn = patch("rpdk.core.init.input_typename", return_value=type_name)
patch_l = patch("rpdk.core.init.input_language", return_value=language)

Expand All @@ -45,23 +39,55 @@ def test_init_method_interactive_language():
mock_l.assert_called_once_with()

mock_project.load_settings.assert_called_once_with()
mock_project.init.assert_called_once_with(type_name, language)
mock_project.init.assert_called_once_with(
type_name,
language,
{
"use_docker": args.use_docker,
"namespace": args.namespace,
"codegen_template_path": args.codegen_model,
"importpath": args.import_path,
},
)
mock_project.generate.assert_called_once_with()


def test_init_method_noninteractive_language():
type_name = object()
def test_init_method_noninteractive():
add_dummy_language_plugin()

args = get_args()
mock_project, patch_project = get_mock_project()

patch_tn = patch("rpdk.core.init.input_typename")
Copy link
Contributor

Choose a reason for hiding this comment

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

do these need to be patched if we are going the non-interactive route?

patch_l = patch("rpdk.core.init.input_language")
Copy link
Contributor

Choose a reason for hiding this comment

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

same here. no reason to patch if we are just asserting we aren't calling


with patch_project, patch_tn as mock_tn, patch_l as mock_l:
init(args)

mock_tn.assert_not_called()
mock_l.assert_not_called()

mock_project.load_settings.assert_called_once_with()
mock_project.init.assert_called_once_with(
args.type_name,
args.language,
{
"use_docker": args.use_docker,
"namespace": args.namespace,
"codegen_template_path": args.codegen_model,
"importpath": args.import_path,
},
)
mock_project.generate.assert_called_once_with()


args = Mock(spec_set=["force", "language"])
args.force = False
args.language = "rust1.39"
def test_init_method_noninteractive_invalid_type_name():
Copy link
Contributor

Choose a reason for hiding this comment

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

confused on this. how does init reach project init method if the typename is invalid?

add_dummy_language_plugin()
type_name = object()

mock_project = Mock(spec=Project)
mock_project.load_settings.side_effect = FileNotFoundError
mock_project.settings_path = ""
mock_project.root = Path(".")
args = get_args(type_name=False)
mock_project, patch_project = get_mock_project()

patch_project = patch("rpdk.core.init.Project", return_value=mock_project)
patch_tn = patch("rpdk.core.init.input_typename", return_value=type_name)
patch_l = patch("rpdk.core.init.input_language")

Expand All @@ -72,7 +98,45 @@ def test_init_method_noninteractive_language():
mock_l.assert_not_called()

mock_project.load_settings.assert_called_once_with()
mock_project.init.assert_called_once_with(type_name, args.language)
mock_project.init.assert_called_once_with(
type_name,
args.language,
{
"use_docker": args.use_docker,
"namespace": args.namespace,
"codegen_template_path": args.codegen_model,
"importpath": args.import_path,
},
)
mock_project.generate.assert_called_once_with()


def test_init_method_noninteractive_invalid_language():
language = object()

args = get_args(language=False)
mock_project, patch_project = get_mock_project()

patch_tn = patch("rpdk.core.init.input_typename")
patch_l = patch("rpdk.core.init.input_language", return_value=language)

with patch_project, patch_tn as mock_tn, patch_l as mock_l:
init(args)

mock_tn.assert_not_called()
mock_l.assert_called_once_with()

mock_project.load_settings.assert_called_once_with()
mock_project.init.assert_called_once_with(
args.type_name,
language,
{
"use_docker": args.use_docker,
"namespace": args.namespace,
"codegen_template_path": args.codegen_model,
"importpath": args.import_path,
},
)
mock_project.generate.assert_called_once_with()


Expand Down
57 changes: 57 additions & 0 deletions tests/utils.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,13 @@
import os
from contextlib import contextmanager
from io import BytesIO
from pathlib import Path
from random import sample
from unittest.mock import Mock, patch

import pkg_resources

from rpdk.core.project import Project

CONTENTS_UTF8 = "💣"

Expand Down Expand Up @@ -67,6 +73,57 @@ def chdir(path):
os.chdir(old)


def add_dummy_language_plugin():
distribution = pkg_resources.Distribution(__file__)
entry_point = pkg_resources.EntryPoint.parse(
"dummy = rpdk.dummy:DummyLanguagePlugin", dist=distribution
)
distribution._ep_map = { # pylint: disable=protected-access
"rpdk.v1.languages": {"dummy": entry_point}
}
pkg_resources.working_set.add(distribution)


def get_mock_project():
mock_project = Mock(spec=Project)
mock_project.load_settings.side_effect = FileNotFoundError
mock_project.settings_path = ""
mock_project.root = Path(".")

patch_project = patch("rpdk.core.init.Project", return_value=mock_project)

return (mock_project, patch_project)


def get_args(interactive=False, language=True, type_name=True):
args = Mock(
spec_set=[
"force",
"language",
"type_name",
"use_docker",
"namespace",
"codegen_model",
"import_path",
]
)
args.force = False
args.language = (
None if interactive else ("dummy" if language else "invalid_language")
Copy link
Contributor

Choose a reason for hiding this comment

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

same here as below. no need to obfuscate what is going on

)
args.type_name = (
None if interactive else ("Test::Test::Test" if type_name else "Test")
Copy link
Contributor

Choose a reason for hiding this comment

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

let's just pass in a type name into this get_args util instead of passing in boolean that sets between two values. makes the function more straightforward

)

# The arguments below will only be tested by the plugins.
args.use_docker = None
args.namespace = "tested.by.the.plugin"
args.codegen_model = None
args.import_path = None

return args


class UnclosingBytesIO(BytesIO):
_was_closed = False

Expand Down