Skip to content

Commit

Permalink
fix: allow hook merging into dicts (#107) and fix index issue with me…
Browse files Browse the repository at this point in the history
…rging into lists
  • Loading branch information
robcxyz committed Jun 15, 2023
1 parent 41218fa commit 2bccc8c
Show file tree
Hide file tree
Showing 7 changed files with 94 additions and 33 deletions.
70 changes: 42 additions & 28 deletions tackle/parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,6 @@ def enrich_hook(
)
hook = get_public_or_private_hook(context=context, hook_type=hook.hook_type)


# Handle args
for n, arg in enumerate(args):
# If help and last arg
Expand Down Expand Up @@ -262,17 +261,25 @@ def merge_output(

if append_hook_value:
if isinstance(key_path[-3], bytes):
set_key(context=context, value=hook_output_value, key_path=key_path[:-1])
# We are merging into a list so we need to keep track of the starting
# index from which we are merging into, the incremented position.
incremented_position = encode_list_index(
decode_list_index(key_path[-3]) + decode_list_index(key_path[-1])
)
tmp_key_path = key_path[:-3] + [incremented_position] + [key_path[-2]]
set_key(context=context, value=hook_output_value, key_path=tmp_key_path)
elif isinstance(hook_output_value, (str, int, float, bool)):
raise exceptions.AppendMergeException(
"Can't merge str/int/float/bool into dict from for loop.",
context=context,
) from None
elif isinstance(hook_output_value, dict):
# We are merging into a dict
for k, v in hook_output_value.items():
tmp_key_path = key_path[:-3] + [key_path[-2]] + [k]
set_key(context=context, value=v, key_path=tmp_key_path)
else:
# TODO: This needs fixing for merging from loop into dict
# https://github.com/robcxyz/tackle/issues/107
tmp_key_path = key_path[:-3] + [key_path[-2]]
set_key(context=context, value=hook_output_value, key_path=tmp_key_path)
raise NotImplementedError("Please raise issue with example if seeing this.")
return

# Can't merge into top level keys without merging k/v individually
Expand Down Expand Up @@ -1336,7 +1343,7 @@ def parse_tmp_context(context: Context, element: Any, existing_context: dict):


def get_complex_field(
field: Any,
field: Any,
) -> Type:
"""
Takes an input field such as `list[str]` which can include uncalled hooks and calls
Expand All @@ -1355,9 +1362,9 @@ def get_complex_field(


def function_walk(
self: 'Context',
input_element: Union[list, dict],
return_: Union[list, dict] = None,
self: 'Context',
input_element: Union[list, dict],
return_: Union[list, dict] = None,
) -> Any:
"""
Walk an input_element for a function and either return the whole context or one or
Expand Down Expand Up @@ -1452,9 +1459,9 @@ def function_walk(


def parse_function_type(
context: Context,
type_str: str,
func_name: str,
context: Context,
type_str: str,
func_name: str,
):
"""
Parse the `type` field within a declarative hook and use recursion to parse the
Expand All @@ -1473,7 +1480,8 @@ def parse_function_type(
context=context,
type_str=arg,
func_name=func_name,
) for arg in re.split(r',(?![^[\]]*])', type_args_str)
)
for arg in re.split(r',(?![^[\]]*])', type_args_str)
]
# Get base type
base_type = parse_function_type(
Expand All @@ -1491,7 +1499,8 @@ def parse_function_type(
else:
raise exceptions.MalformedFunctionFieldException(
"The type `Optional` only takes one arg.",
context=context, function_name=func_name,
context=context,
function_name=func_name,
) from None
else:
return base_type[tuple(type_args)]
Expand All @@ -1507,7 +1516,9 @@ def parse_function_type(
except AttributeError:
raise exceptions.MalformedFunctionFieldException(
f"The type `{type_str}` is not recognized. Must be in python's "
f"`typing` module.", context=context, function_name=func_name,
f"`typing` module.",
context=context,
function_name=func_name,
) from None
return type_
elif isinstance(hook, LazyBaseFunction):
Expand All @@ -1524,10 +1535,10 @@ def parse_function_type(


def function_extends_merge_hook(
context: 'Context',
func_name: str,
func_dict: dict,
extends: str,
context: 'Context',
func_name: str,
func_dict: dict,
extends: str,
):
base_hook = get_hook(
context=context,
Expand All @@ -1548,9 +1559,9 @@ def function_extends_merge_hook(


def function_extends(
context: 'Context',
func_name: str,
func_dict: dict,
context: 'Context',
func_name: str,
func_dict: dict,
):
"""
Implement the `extends` functionality which takes either a string reference or list
Expand All @@ -1577,14 +1588,16 @@ def function_extends(
)
raise exceptions.MalformedFunctionFieldException(
"The field `extends` can only be a string or list of string references to "
"hooks to merge together.", context=context, function_name=func_name,
"hooks to merge together.",
context=context,
function_name=func_name,
) from None


def create_function_model(
context: 'Context',
func_name: str,
func_dict: dict,
context: 'Context',
func_name: str,
func_dict: dict,
) -> Type[BaseFunction]:
"""Create a model from the function input dict."""
if func_name.endswith(('<-', '<_')):
Expand Down Expand Up @@ -1676,7 +1689,8 @@ def create_function_model(
if 'type' in v:
raise exceptions.MalformedFunctionFieldException(
'Enums are implicitly typed.',
context=context, function_name=func_name
context=context,
function_name=func_name,
)
enum_type = enum.Enum(k, {i: i for i in v['enum']})
if 'default' in v:
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
resources:
name: operator
type: helm
# Should error since it is a value, not a map
do some merge->: literal foo-{{index}} --for [1,2,3] --merge
4 changes: 4 additions & 0 deletions tests/parser/base-method-fixtures/merge-dict.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
resources:
name: operator
type: helm
do some merge->: literal {'foo':'foo'} --merge
6 changes: 6 additions & 0 deletions tests/parser/base-method-fixtures/merge-list-dict.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
resources:
name: operator
do some merge->: literal [1,2,3] --merge
# This overwrites resources -> len = 2
# https://github.com/sudoblockio/tackle/issues/163
do another merge->: literal [2,3] --merge
6 changes: 6 additions & 0 deletions tests/parser/base-method-fixtures/merge-list-loop-dict.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
resources:
- name: operator
type: helm
- name: Cluster role
type: manifest
- ->: literal {'foo-{{index}}':'foo-{{index}}'} --for [1,2,3] --merge
6 changes: 6 additions & 0 deletions tests/parser/base-method-fixtures/merge-list-loop-value.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
resources:
- name: operator
type: helm
- name: Cluster role
type: manifest
- ->: literal foo-{{index}} --for [1,2,3] --merge
34 changes: 29 additions & 5 deletions tests/parser/test_parser_base_methods.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,13 @@ def fixture_dir(chdir):
chdir("base-method-fixtures")


def test_parser_methods_merge_dict(fixture_dir):
"""Validate merging a dict into a dict."""
output = tackle('merge-dict.yaml')
assert output['resources']['name'] == 'operator'
assert output['resources']['foo'] == 'foo'


def test_parser_methods_merge_list_value(fixture_dir):
"""Validate that when in a list, running a hook with a merge overwrites the list."""
# Note: this is kind of strong... But what else is it supposed to mean? Don't use
Expand All @@ -27,13 +34,22 @@ def test_parser_methods_merge_list_value(fixture_dir):
assert output['resources'] == 'foo'


def test_parser_methods_merge_list_loop(fixture_dir):
def test_parser_methods_merge_list_loop_value(fixture_dir):
"""
Validate that when in a list, running a hook in a for loop with a merge appends to
the list.
"""
output = tackle('merge-list-loop.yaml')
output = tackle('merge-list-loop-value.yaml')
assert len(output['resources']) == 5
assert output['resources'][2] == 'foo-0'


def test_parser_methods_merge_list_loop_dict(fixture_dir):
"""Same as above but with a dict."""
output = tackle('merge-list-loop-dict.yaml')
assert len(output['resources']) == 5
assert output['resources'][2]['foo-0'] == 'foo-0'
assert output['resources'][3]['foo-1'] == 'foo-1'


def test_parser_methods_merge_dict_loop_dict(fixture_dir):
Expand All @@ -42,9 +58,9 @@ def test_parser_methods_merge_dict_loop_dict(fixture_dir):
keys to the output.
"""
output = tackle('merge-dict-loop-dict.yaml')
assert output['resources']['foo-2']
# TODO: Associated with https://github.com/robcxyz/tackle/issues/107
# assert output['resources']['foo-1']
assert output['resources']['foo-2'] == 'foo-2'
assert output['resources']['name'] == 'operator'
assert output['resources']['foo-1'] == 'foo-1'


def test_parser_methods_merge_dict_loop_exception(fixture_dir):
Expand All @@ -56,6 +72,14 @@ def test_parser_methods_merge_dict_loop_exception(fixture_dir):
tackle('merge-dict-loop-exception.yaml')


# def test_parser_methods_merge_list_dict(fixture_dir):
# """Validate when we merge a list into a dict that it overwrites the contents."""
# output = tackle('merge-list-dict.yaml')
# # TODO: Determine how to merge twice into a list
# # https://github.com/sudoblockio/tackle/issues/163
# assert output


def test_parser_methods_try(fixture_dir):
"""Use try which should not have any output"""
output = tackle('method-try.yaml')
Expand Down

0 comments on commit 2bccc8c

Please sign in to comment.