Skip to content

Commit

Permalink
[cli] Add create-resources flag and default to not creating external …
Browse files Browse the repository at this point in the history
…resources
  • Loading branch information
fallonchen committed Sep 10, 2020
1 parent a7a2b0f commit 9d17c1b
Show file tree
Hide file tree
Showing 4 changed files with 60 additions and 22 deletions.
5 changes: 1 addition & 4 deletions cli/src/klio_cli/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -248,10 +248,7 @@ def _validate_job_name(ctx, param, value):

@job.command(
"create",
help=(
"Create a new Klio job. This generates the necessary files as well as "
"the required Google Cloud resources."
),
help=("Create the necessary files for a new Klio job."),
context_settings=dict(
ignore_unknown_options=True,
allow_extra_args=True,
Expand Down
42 changes: 37 additions & 5 deletions cli/src/klio_cli/commands/job/create.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,5 @@
# -*- coding: utf-8 -*-
# Copyright 2019 Spotify AB

from __future__ import absolute_import

import datetime
import io
import logging
Expand Down Expand Up @@ -40,6 +37,7 @@
"worker_machine_type": "n1-standard-2",
"python_version": "3.6",
"use_fnapi": True,
"create_resources": False,
}


Expand Down Expand Up @@ -219,6 +217,7 @@ def _get_worker_image():
else:
use_fnapi = DEFAULTS["use_fnapi"]

create_resources = self._get_create_resources(kwargs)
default_exps = DEFAULTS["experiments"].split(",")
if not use_fnapi:
default_exps = [e for e in default_exps if e != "beam_fn_api"]
Expand Down Expand Up @@ -304,10 +303,39 @@ def _get_worker_image():
"job_options": job_context,
"python_version": python_version,
"use_fnapi": use_fnapi,
"create_resources": create_resources,
}

return context, create_dockerfile

def _get_create_resources(self, kwargs, user_input=False):
if "create_resources" in kwargs:
create_resources = kwargs.get("create_resources")
if create_resources:
create_resources = str(create_resources).lower() in (
"y",
"true",
"yes",
)
else: # then it was used as a flag
create_resources = True
else:
if user_input:
create_resources = click.prompt(
"Create topics, buckets, and dashboards? [Y/n]",
type=click.Choice(["y", "Y", "n", "N"]),
default="n"
if DEFAULTS["create_resources"] is False
else "y",
show_choices=False,
show_default=False, # shown in prompt
)
create_resources = create_resources.lower() == "y"
else:
create_resources = DEFAULTS["create_resources"]

return create_resources

def _get_dependencies_from_user_inputs(self):
dependencies = []
while True:
Expand Down Expand Up @@ -360,6 +388,8 @@ def _get_context_from_user_inputs(self, kwargs):
)
use_fnapi = use_fnapi.lower() == "y"

create_resources = self._get_create_resources(kwargs, user_input=True)

# TODO should this even be an option? run-job will break if so.
# TODO: figure out if we should expose `experiments` to the user, or
# if it's okay to always assume `beam_fn_api` is the only
Expand Down Expand Up @@ -523,6 +553,7 @@ def _get_context_from_user_inputs(self, kwargs):
"job_options": job_context,
"python_version": python_version,
"use_fnapi": use_fnapi,
"create_resources": create_resources,
}
return context, create_dockerfile

Expand Down Expand Up @@ -597,8 +628,9 @@ def _parse_unknown_args(self, user_args):
return parsed_args

def _create_external_resources(self, context):
gcp_setup.create_topics_and_buckets(context)
gcp_setup.create_stackdriver_dashboard(context)
if context["create_resources"]:
gcp_setup.create_topics_and_buckets(context)
gcp_setup.create_stackdriver_dashboard(context)

def create(self, unknown_args, known_kwargs, output_dir):
unknown_args = self._parse_unknown_args(unknown_args)
Expand Down
30 changes: 21 additions & 9 deletions cli/tests/commands/job/test_create.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,5 @@
# -*- coding: utf-8 -*-
# Copyright 2019 Spotify AB

from __future__ import absolute_import

import datetime
import os

Expand Down Expand Up @@ -154,6 +151,7 @@ def default_context():
"job_name": "test-job",
"python_version": "36",
"use_fnapi": True,
"create_resources": False,
"pipeline_options": {
"project": "test-gcp-project",
"region": "europe-west1",
Expand Down Expand Up @@ -307,6 +305,7 @@ def test_create_dockerfile(use_fnapi, tmpdir, job):
},
"python_version": "36",
"use_fnapi": use_fnapi,
"create_resources": False,
}

job._create_dockerfile(env, context, output_dir.strpath)
Expand Down Expand Up @@ -438,6 +437,7 @@ def context_overrides():
],
"python_version": "37",
"use_fnapi": "n",
"create_resources": "n",
}


Expand Down Expand Up @@ -482,6 +482,7 @@ def expected_overrides():
},
"python_version": "37",
"use_fnapi": False,
"create_resources": False,
}


Expand Down Expand Up @@ -615,6 +616,7 @@ def test_get_context_from_user_inputs(
prompt_side_effect = [
"europe-west1",
"Y",
"n",
["beam_fn_api"],
2,
2,
Expand Down Expand Up @@ -670,6 +672,7 @@ def test_get_context_from_user_inputs(
context.pop("job_name")
context["pipeline_options"].pop("project")
context["use_fnapi"] = True
context["create_resources"] = False
assert context == ret_context
assert ret_dockerfile

Expand Down Expand Up @@ -910,14 +913,12 @@ def test_parse_unknown_args(unknown_args, expected, job):

@pytest.mark.parametrize("create_dockerfile", (True, False))
@pytest.mark.parametrize("use_fnapi", (True, False))
def test_create(use_fnapi, create_dockerfile, mocker, caplog, job):
context = {"job_name": "test-job", "use_fnapi": use_fnapi}
@pytest.mark.parametrize("create_resources", (True, False))
def test_create(use_fnapi, create_dockerfile, create_resources, mocker, caplog, job):
context = {"job_name": "test-job", "use_fnapi": use_fnapi, "create_resources": create_resources}

mock_get_user_input = mocker.patch.object(job, "_get_user_input")
mock_get_user_input.return_value = (context, create_dockerfile)
mock_create_external_resources = mocker.patch.object(
job, "_create_external_resources"
)
mock_get_environment = mocker.patch.object(job, "_get_environment")
mock_create_job_dir = mocker.patch.object(job, "_create_job_directory")
mock_create_job_config = mocker.patch.object(job, "_create_job_config")
Expand All @@ -929,6 +930,10 @@ def test_create(use_fnapi, create_dockerfile, mocker, caplog, job):
mock_create_dockerfile = mocker.patch.object(job, "_create_dockerfile")
mock_create_readme = mocker.patch.object(job, "_create_readme")

mock_create_topics = mocker.patch.object(create.gcp_setup, "create_topics_and_buckets")
mock_create_stackdriver = mocker.patch.object(create.gcp_setup, "create_stackdriver_dashboard")


unknown_args = ("--foo", "bar")
known_args = {
"job_name": "test-job",
Expand All @@ -941,7 +946,6 @@ def test_create(use_fnapi, create_dockerfile, mocker, caplog, job):
known_args["foo"] = "bar"
mock_get_user_input.assert_called_once_with(known_args)

mock_create_external_resources.assert_called_once_with(context)
mock_get_environment.assert_called_once_with()

ret_env = mock_get_environment.return_value
Expand All @@ -963,6 +967,14 @@ def test_create(use_fnapi, create_dockerfile, mocker, caplog, job):
mock_create_no_fnapi_files.assert_called_once_with(
ret_env, context, output_dir
)

if create_resources:
mock_create_topics.assert_called_once_with(context)
mock_create_stackdriver.assert_called_once_with(context)
else:
mock_create_topics.assert_not_called()
mock_create_stackdriver.assert_not_called()

mock_create_reqs_files.assert_called_once_with(
ret_env, context, output_dir
)
Expand Down
5 changes: 1 addition & 4 deletions cli/tests/test_cli.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,5 @@
# -*- coding: utf-8 -*-
# Copyright 2019 Spotify AB

from __future__ import absolute_import

import copy
import os

Expand Down Expand Up @@ -173,6 +170,7 @@ def minimal_config_data():
def minimal_mock_klio_config(
mock_klio_config, config_file, minimal_config_data
):

mock_klio_config.setup(minimal_config_data, config_file)
return mock_klio_config

Expand Down Expand Up @@ -1044,7 +1042,6 @@ def test_profile_memory(runner, mocker, minimal_mock_klio_config):

def test_profile_memory_per_line(runner, mocker, minimal_mock_klio_config):
mock_profile = mocker.patch.object(cli, "_profile")

result = runner.invoke(cli.profile_memory_per_line, [])

assert_execution_success(result)
Expand Down

0 comments on commit 9d17c1b

Please sign in to comment.