From ff25a28ba7d93f88a47c32f5ecf678aa45548b6d Mon Sep 17 00:00:00 2001 From: Nicholas Cilfone <23509131+ncilfone@users.noreply.github.com> Date: Mon, 6 Mar 2023 11:39:33 -0500 Subject: [PATCH 01/14] added docs for __map__ post init hooks. --- spock/backend/config.py | 31 +++++++-- spock/utils.py | 23 ++++++- tests/base/test_maps.py | 61 +++++++++++++++++ tests/base/test_post_hooks.py | 37 ++++++----- website/docs/advanced_features/Post-Hooks.md | 70 ++++++++++++++++++-- 5 files changed, 197 insertions(+), 25 deletions(-) create mode 100644 tests/base/test_maps.py diff --git a/spock/backend/config.py b/spock/backend/config.py index 2e50e627..90e61225 100644 --- a/spock/backend/config.py +++ b/spock/backend/config.py @@ -11,7 +11,7 @@ from spock.backend.typed import katra from spock.exceptions import _SpockInstantiationError, _SpockUndecoratedClass -from spock.utils import _is_spock_instance, vars_dict_non_dunder +from spock.utils import _is_spock_instance, contains_return, vars_dict_non_dunder def _base_attr(cls, kw_only, make_init, dynamic): @@ -150,10 +150,31 @@ def _process_class(cls, kw_only: bool, make_init: bool, dynamic: bool): """ # Handles the MRO and gets old annotations bases, attrs_dict, merged_annotations = _base_attr(cls, kw_only, make_init, dynamic) - # Copy over the post init function -- borrow a bit from attrs library to add the __post__hook__ method to the - # init call via `"__attrs_post_init__"` - if hasattr(cls, "__post_hook__"): - attrs_dict.update({"__attrs_post_init__": cls.__post_hook__}) + # Copy over the post init function -- borrow a bit from attrs library to add the + # __post__hook__ method and/or the __maps__ method (via a shim method) to the init + # call via `"__attrs_post_init__"` + if hasattr(cls, "__post_hook__") or hasattr(cls, "__maps__"): + # Force the post_hook function to have no explict return + if hasattr(cls, "__post_hook__") and contains_return(cls.__post_hook__): + raise _SpockInstantiationError( + f"__post_hook__ function contains an explict return. This function " + f"cannot return any values (i.e. requires an implicit None return)" + ) + if hasattr(cls, "__maps__") and not contains_return(cls.__maps__): + raise _SpockInstantiationError( + f"__maps__ function is missing an explict return. This function " + f"needs to explicitly return any type of values" + ) + + # Create a shim function to combine __post_hook__ and __maps__ + def __shim__(self): + if hasattr(cls, "__post_hook__"): + cls.__post_hook__(self) + if hasattr(cls, "__maps__"): + object.__setattr__(self, "_maps", cls.__maps__(self)) + + # Map the __shim__ function into __attrs_post_init__ + attrs_dict.update({"__attrs_post_init__": __shim__}) # Dynamically make an attr class obj = attr.make_class( name=cls.__name__, diff --git a/spock/utils.py b/spock/utils.py index 7c0bb86a..e24d0860 100644 --- a/spock/utils.py +++ b/spock/utils.py @@ -6,17 +6,19 @@ """Utility functions for Spock""" import ast +import inspect import os import random import socket import subprocess import sys +import textwrap from argparse import _ArgumentGroup from enum import EnumMeta from math import isclose from pathlib import Path from time import localtime, strftime -from typing import Any, Dict, List, Tuple, Type, TypeVar, Union +from typing import Any, Callable, Dict, List, Tuple, Type, TypeVar, Union from warnings import warn import attr @@ -28,6 +30,25 @@ minor = sys.version_info.minor +def contains_return(func: Callable): + """Checks if a function/callable has an explict return def + + Args: + func: function to check for direct return + + Returns: + boolean if defined return is found + + References: + https://stackoverflow.com/questions/48232810/python-check-if-function-has-return-statement + + """ + return any( + isinstance(node, ast.Return) + for node in ast.walk(ast.parse(textwrap.dedent(inspect.getsource(func)))) + ) + + def vars_dict_non_dunder(__obj: object): """Gets the user defined attributes from a base object class diff --git a/tests/base/test_maps.py b/tests/base/test_maps.py new file mode 100644 index 00000000..52fcfe97 --- /dev/null +++ b/tests/base/test_maps.py @@ -0,0 +1,61 @@ +# -*- coding: utf-8 -*- +import sys + +from typing import List, Tuple, Optional + +import pytest + +from spock import spock +from spock import SpockBuilder +from spock.exceptions import _SpockInstantiationError + + +class DummyClass: + def __init__(self, value): + self.value = value + + +class TestMaps: + def test_return_raise(self, monkeypatch, tmp_path): + with monkeypatch.context() as m: + m.setattr( + sys, + "argv", + [""], + ) + with pytest.raises(_SpockInstantiationError): + + @spock + class FailReturnConfig: + val_1: float = 0.5 + + def __maps__(self): + print(self.val_1) + + config = SpockBuilder( + FailReturnConfig, + desc="Test Builder", + ) + config.generate() + + def test_map_return(self, monkeypatch, tmp_path): + with monkeypatch.context() as m: + m.setattr( + sys, + "argv", + [""], + ) + + @spock + class ReturnConfig: + val_1: float = 0.5 + + def __maps__(self): + return DummyClass(value=self.val_1) + + config = SpockBuilder( + ReturnConfig, + desc="Test Builder", + ) + configs = config.generate() + assert configs.ReturnConfig._maps.value == configs.ReturnConfig.val_1 diff --git a/tests/base/test_post_hooks.py b/tests/base/test_post_hooks.py index 1157887d..245b3e81 100644 --- a/tests/base/test_post_hooks.py +++ b/tests/base/test_post_hooks.py @@ -1,5 +1,4 @@ # -*- coding: utf-8 -*- -# -*- coding: utf-8 -*- import sys from typing import List, Tuple, Optional @@ -160,8 +159,29 @@ def __post_hook__(self): class TestPostHooks: + def test_return_raise(self, monkeypatch, tmp_path): + with monkeypatch.context() as m: + m.setattr( + sys, + "argv", + [""], + ) + with pytest.raises(_SpockInstantiationError): + + @spock + class FailReturnConfig: + val_1: float = 0.5 + + def __post_hook__(self): + return self.val_1 + + config = SpockBuilder( + FailReturnConfig, + desc="Test Builder", + ) + config.generate() + def test_sum_none_fail_config(self, monkeypatch, tmp_path): - """Test serialization/de-serialization""" with monkeypatch.context() as m: m.setattr( sys, @@ -176,7 +196,6 @@ def test_sum_none_fail_config(self, monkeypatch, tmp_path): config.generate() def test_sum_not_equal_config(self, monkeypatch, tmp_path): - """Test serialization/de-serialization""" with monkeypatch.context() as m: m.setattr( sys, @@ -191,7 +210,6 @@ def test_sum_not_equal_config(self, monkeypatch, tmp_path): config.generate() def test_eq_len_two_len_fail(self, monkeypatch, tmp_path): - """Test serialization/de-serialization""" with monkeypatch.context() as m: m.setattr( sys, @@ -206,7 +224,6 @@ def test_eq_len_two_len_fail(self, monkeypatch, tmp_path): config.generate() def test_eq_len_none_fail(self, monkeypatch, tmp_path): - """Test serialization/de-serialization""" with monkeypatch.context() as m: m.setattr( sys, @@ -221,7 +238,6 @@ def test_eq_len_none_fail(self, monkeypatch, tmp_path): config.generate() def test_eq_len_none(self, monkeypatch, tmp_path): - """Test serialization/de-serialization""" with monkeypatch.context() as m: m.setattr( sys, @@ -235,7 +251,6 @@ def test_eq_len_none(self, monkeypatch, tmp_path): config.generate() def test_within_low(self, monkeypatch, tmp_path): - """Test serialization/de-serialization""" with monkeypatch.context() as m: m.setattr( sys, @@ -250,7 +265,6 @@ def test_within_low(self, monkeypatch, tmp_path): config.generate() def test_within_high(self, monkeypatch, tmp_path): - """Test serialization/de-serialization""" with monkeypatch.context() as m: m.setattr( sys, @@ -265,7 +279,6 @@ def test_within_high(self, monkeypatch, tmp_path): config.generate() def test_within_none(self, monkeypatch, tmp_path): - """Test serialization/de-serialization""" with monkeypatch.context() as m: m.setattr( sys, @@ -310,7 +323,6 @@ def test_gt_none(self, monkeypatch, tmp_path): config.generate() def test_ge(self, monkeypatch, tmp_path): - """Test serialization/de-serialization""" with monkeypatch.context() as m: m.setattr( sys, @@ -325,7 +337,6 @@ def test_ge(self, monkeypatch, tmp_path): config.generate() def test_ge_none(self, monkeypatch, tmp_path): - """Test serialization/de-serialization""" with monkeypatch.context() as m: m.setattr( sys, @@ -340,7 +351,6 @@ def test_ge_none(self, monkeypatch, tmp_path): config.generate() def test_lt(self, monkeypatch, tmp_path): - """Test serialization/de-serialization""" with monkeypatch.context() as m: m.setattr( sys, @@ -355,7 +365,6 @@ def test_lt(self, monkeypatch, tmp_path): config.generate() def test_lt_none(self, monkeypatch, tmp_path): - """Test serialization/de-serialization""" with monkeypatch.context() as m: m.setattr( sys, @@ -370,7 +379,6 @@ def test_lt_none(self, monkeypatch, tmp_path): config.generate() def test_le(self, monkeypatch, tmp_path): - """Test serialization/de-serialization""" with monkeypatch.context() as m: m.setattr( sys, @@ -385,7 +393,6 @@ def test_le(self, monkeypatch, tmp_path): config.generate() def test_le_none(self, monkeypatch, tmp_path): - """Test serialization/de-serialization""" with monkeypatch.context() as m: m.setattr( sys, diff --git a/website/docs/advanced_features/Post-Hooks.md b/website/docs/advanced_features/Post-Hooks.md index 015f3765..d275ad52 100644 --- a/website/docs/advanced_features/Post-Hooks.md +++ b/website/docs/advanced_features/Post-Hooks.md @@ -1,10 +1,12 @@ # Post Initialization Hooks -`spock` supports post initialization (post-init) hooks via the `__post_hook__` method within a `@spock` decorated class. -These methods are automatically called after the `Spockspace` object is created thus allow the user to do any post -instantiation work (such as validation). +`spock` supports post initialization (post-init) hooks via the `__post_hook__` and +`__map__` methods within a `@spock` decorated class. +These methods are automatically called after the `Spockspace` object is created thus +allow the user to do any post instantiation work (such as validation) or map parameter +values to object/class instantiation. -### Creating and Using Post-Init Hooks +### Post-Init Hooks Simply add the `__post_hook__` method to your `@spock` decorated class: @@ -40,6 +42,66 @@ since the value of `my_value` does not fall within the given bounds. Also notice parameters within the `__post_hook__` methods and use them in custom functions (here we use `lower_bound` and `check_sum` to do some validation comparisons) + +### Map Hooks + +Simply add the `__map__` method to your `@spock` decorated class. This method must +contain an explict return. The return(s) of the `__map__` method will be mapped to the +`_map` variable within the instantiated class> For instance: + + +```python +# -*- coding: utf-8 -*- +from spock import spock, SpockBuilder +from sagemaker.processing import ProcessingInput + +from enum import Enum +from typing import Optional + +class UploadMode(Enum): + end_of_job = "EndOfJob" + continuous = "Continuous" + + +@spock +class SMBaseOutputConfig: + name: Optional[str] = None + source: str + destination: str + upload_mode: UploadMode = UploadMode.end_of_job + + +@spock +class S3ScratchConfig(SMBaseOutputConfig): + ... + + def __post_hook__(self): + print(self.name) + + def __maps__(self): + return ProcessingInput( + input_name=self.name, + source=self.source, + destination=self.destination + ) + +def main(): + # Get configs + configs = SpockBuilder( + S3ScratchConfig, + desc="Configs for testing containers on SM", + lazy=True + ).generate() + print(configs.S3ScatchConfig._maps) + +``` + +The `__map__` method will be triggered post-instantiation. This means that the parameter +values defined within the `S3ScratchConfig` class will get mapped into the creation of +the `ProcessingInput` instance. Therefore, the return of `_maps` will be the +instantiated `ProcessingInput` object with the `spock` defined parameters. + + ### Common Post Initialization Hooks `spock` provides some common validators (in the `utils` module) that would be typically run as post-init From 28b307a022faf3a1e65995855d595920b6f3ffff Mon Sep 17 00:00:00 2001 From: Nicholas Cilfone <23509131+ncilfone@users.noreply.github.com> Date: Mon, 6 Mar 2023 11:52:16 -0500 Subject: [PATCH 02/14] removing the protected attribute from the repr print --- spock/backend/wrappers.py | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/spock/backend/wrappers.py b/spock/backend/wrappers.py index d0c245cc..51021dc3 100644 --- a/spock/backend/wrappers.py +++ b/spock/backend/wrappers.py @@ -22,10 +22,17 @@ def __init__(self, **kwargs): @property def __repr_dict__(self): - """Handles making a clean dict to hind the salt and key on print""" - return { + """Handles making a clean dict to hide the salt and key on print""" + clean_dict = { k: v for k, v in self.__dict__.items() if k not in {"__key__", "__salt__"} } + repr_dict = {} + for k, v in clean_dict.items(): + repr_dict.update( + {k: {ik: iv for ik, iv in vars(v).items() if not ik.startswith("_")}} + ) + + return repr_dict def __repr__(self): """Overloaded repr to pretty print the spock object""" From 628571d5ae6db41dd49ab187cca8660f7d914a4f Mon Sep 17 00:00:00 2001 From: Nicholas Cilfone <23509131+ncilfone@users.noreply.github.com> Date: Mon, 6 Mar 2023 12:45:16 -0500 Subject: [PATCH 03/14] strip protected attributes from save method --- spock/backend/saver.py | 7 ++----- spock/backend/wrappers.py | 5 ++++- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/spock/backend/saver.py b/spock/backend/saver.py index 1b248656..ac48c72a 100644 --- a/spock/backend/saver.py +++ b/spock/backend/saver.py @@ -216,6 +216,8 @@ def __call__(self, *args, **kwargs): return AttrSaver(*args, **kwargs) def _clean_up_values(self, payload: Spockspace, remove_crypto: bool = True) -> Dict: + # Strip annotations and protected attributes + payload = payload.clean # Dictionary to recursively write to out_dict = {} # All of the classes are defined at the top level @@ -227,11 +229,6 @@ def _clean_up_values(self, payload: Spockspace, remove_crypto: bool = True) -> D clean_dict = self._clean_output(out_dict) # Clip any empty dictionaries clean_dict = {k: v for k, v in clean_dict.items() if len(v) > 0} - if remove_crypto: - if "__salt__" in clean_dict: - _ = clean_dict.pop("__salt__") - if "__key__" in clean_dict: - _ = clean_dict.pop("__key__") return clean_dict def _clean_tuner_values(self, payload: Spockspace) -> Dict: diff --git a/spock/backend/wrappers.py b/spock/backend/wrappers.py index 51021dc3..ef628952 100644 --- a/spock/backend/wrappers.py +++ b/spock/backend/wrappers.py @@ -20,6 +20,10 @@ class Spockspace(argparse.Namespace): def __init__(self, **kwargs): super(Spockspace, self).__init__(**kwargs) + @property + def clean(self): + return Spockspace(**self.__repr_dict__) + @property def __repr_dict__(self): """Handles making a clean dict to hide the salt and key on print""" @@ -31,7 +35,6 @@ def __repr_dict__(self): repr_dict.update( {k: {ik: iv for ik, iv in vars(v).items() if not ik.startswith("_")}} ) - return repr_dict def __repr__(self): From 9b6af048298c3f12f6e338fe1693596b47506942 Mon Sep 17 00:00:00 2001 From: Nicholas Cilfone <23509131+ncilfone@users.noreply.github.com> Date: Mon, 6 Mar 2023 14:50:28 -0500 Subject: [PATCH 04/14] handling inhert of hooks from parents --- spock/backend/config.py | 89 ++++++++++++++++---- website/docs/advanced_features/Post-Hooks.md | 6 +- 2 files changed, 75 insertions(+), 20 deletions(-) diff --git a/spock/backend/config.py b/spock/backend/config.py index 90e61225..484e4b52 100644 --- a/spock/backend/config.py +++ b/spock/backend/config.py @@ -6,6 +6,7 @@ """Creates the spock config interface that wraps attr""" import sys +from typing import Dict import attr @@ -27,7 +28,7 @@ def _base_attr(cls, kw_only, make_init, dynamic): dynamic: allows inherited classes to not be @spock decorated Returns: - cls: base spock classes derived from the MRO + bases: all the base classes attrs_dict: the current dictionary of attr.attribute values merged_annotations: dictionary of type annotations @@ -83,6 +84,7 @@ def _base_attr(cls, kw_only, make_init, dynamic): merged_annotations = {**base_annotation, **new_annotations} cls_attrs = set() + hooks = set() # Iterate through the bases first for val in bases: # Get the underlying attribute defs @@ -135,21 +137,26 @@ def _base_attr(cls, kw_only, make_init, dynamic): return bases, attrs_dict, merged_annotations -def _process_class(cls, kw_only: bool, make_init: bool, dynamic: bool): - """Process a given class +def _handle_hooks( + cls, + bases, +): + """Handles creating a single function for all hooks from the given class and + all its parents Args: cls: basic class definition - kw_only: set kwarg only - make_init: make an init function - dynamic: allows inherited classes to not be @spock decorated Returns: - cls with attrs dunder methods added + function that contains all necessary hooks """ - # Handles the MRO and gets old annotations - bases, attrs_dict, merged_annotations = _base_attr(cls, kw_only, make_init, dynamic) + + # Check if the base classes have any hook functions + hooks = [ + val.__attrs_post_init__ for val in bases if hasattr(val, "__attrs_post_init__") + ] + # maps = [val.__maps__ for val in bases if hasattr(val, "__maps__")] # Copy over the post init function -- borrow a bit from attrs library to add the # __post__hook__ method and/or the __maps__ method (via a shim method) to the init # call via `"__attrs_post_init__"` @@ -165,16 +172,63 @@ def _process_class(cls, kw_only: bool, make_init: bool, dynamic: bool): f"__maps__ function is missing an explict return. This function " f"needs to explicitly return any type of values" ) + # if there are parent hooks we need to map them into a function + if len(hooks) > 0: + # Create a shim function to combine __post_hook__ and __maps__ + # in addition to the parental hooks + def __shim__(self): + if hasattr(cls, "__post_hook__"): + cls.__post_hook__(self) + # Call the parents hooks + all_hooks = [val(self) for val in hooks] + # Add in the given hook + if hasattr(cls, "__maps__"): + all_hooks = [cls.__maps__(self)] + all_hooks + elif len(all_hooks) == 1: + all_hooks = all_hooks[0] + # Set maps to the mapped values + object.__setattr__(self, "_maps", all_hooks) + + else: + # Create a shim function to combine __post_hook__ and __maps__ + def __shim__(self): + if hasattr(cls, "__post_hook__"): + cls.__post_hook__(self) + if hasattr(cls, "__maps__"): + object.__setattr__(self, "_maps", cls.__maps__(self)) + return cls.__maps__(self) + else: + return None + + else: - # Create a shim function to combine __post_hook__ and __maps__ def __shim__(self): - if hasattr(cls, "__post_hook__"): - cls.__post_hook__(self) - if hasattr(cls, "__maps__"): - object.__setattr__(self, "_maps", cls.__maps__(self)) + ... + + return __shim__ + - # Map the __shim__ function into __attrs_post_init__ - attrs_dict.update({"__attrs_post_init__": __shim__}) +def _process_class(cls, kw_only: bool, make_init: bool, dynamic: bool): + """Process a given class + + Args: + cls: basic class definition + kw_only: set kwarg only + make_init: make an init function + dynamic: allows inherited classes to not be @spock decorated + + Returns: + cls with attrs dunder methods added + + """ + # Handles the MRO and gets old annotations + bases, attrs_dict, merged_annotations = _base_attr(cls, kw_only, make_init, dynamic) + # if hasattr(cls, "__post_hook__"): + # attrs_dict.update({"__post_hook__": cls.__post_hook__}) + # if hasattr(cls, "__maps__"): + # attrs_dict.update({"__maps__": cls.__maps__}) + # Map the __shim__ function into __attrs_post_init__ + attrs_dict.update({"__attrs_post_init__": _handle_hooks(cls, bases)}) # Dynamically make an attr class obj = attr.make_class( name=cls.__name__, @@ -185,7 +239,8 @@ def __shim__(self): auto_attribs=True, init=make_init, ) - # For each class we dynamically create we need to register it within the system modules for pickle to work + # For each class we dynamically create we need to register it within the system + # modules for pickle to work setattr(sys.modules["spock"].backend.config, obj.__name__, obj) # Swap the __doc__ string from cls to obj obj.__doc__ = cls.__doc__ diff --git a/website/docs/advanced_features/Post-Hooks.md b/website/docs/advanced_features/Post-Hooks.md index d275ad52..3bcb0a47 100644 --- a/website/docs/advanced_features/Post-Hooks.md +++ b/website/docs/advanced_features/Post-Hooks.md @@ -45,8 +45,8 @@ parameters within the `__post_hook__` methods and use them in custom functions ( ### Map Hooks -Simply add the `__map__` method to your `@spock` decorated class. This method must -contain an explict return. The return(s) of the `__map__` method will be mapped to the +Simply add the `__maps__` method to your `@spock` decorated class. This method must +contain an explict return. The return(s) of the `__maps__` method will be mapped to the `_map` variable within the instantiated class> For instance: @@ -96,7 +96,7 @@ def main(): ``` -The `__map__` method will be triggered post-instantiation. This means that the parameter +The `__maps__` method will be triggered post-instantiation. This means that the parameter values defined within the `S3ScratchConfig` class will get mapped into the creation of the `ProcessingInput` instance. Therefore, the return of `_maps` will be the instantiated `ProcessingInput` object with the `spock` defined parameters. From c61f8ede9ab72a7723ddb4463be9921489c06d63 Mon Sep 17 00:00:00 2001 From: Nicholas Cilfone <23509131+ncilfone@users.noreply.github.com> Date: Mon, 6 Mar 2023 14:54:18 -0500 Subject: [PATCH 05/14] fix to clean up of protected attributes --- spock/backend/saver.py | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/spock/backend/saver.py b/spock/backend/saver.py index ac48c72a..63f5443e 100644 --- a/spock/backend/saver.py +++ b/spock/backend/saver.py @@ -216,8 +216,6 @@ def __call__(self, *args, **kwargs): return AttrSaver(*args, **kwargs) def _clean_up_values(self, payload: Spockspace, remove_crypto: bool = True) -> Dict: - # Strip annotations and protected attributes - payload = payload.clean # Dictionary to recursively write to out_dict = {} # All of the classes are defined at the top level @@ -229,7 +227,21 @@ def _clean_up_values(self, payload: Spockspace, remove_crypto: bool = True) -> D clean_dict = self._clean_output(out_dict) # Clip any empty dictionaries clean_dict = {k: v for k, v in clean_dict.items() if len(v) > 0} - return clean_dict + # Clean up annotations + if remove_crypto: + if "__salt__" in clean_dict: + _ = clean_dict.pop("__salt__") + if "__key__" in clean_dict: + _ = clean_dict.pop("__key__") + # Clean up protected attributes + out_dict = {} + for k, v in clean_dict.items(): + cls_dict = {} + for ik, iv in v.items(): + if not ik.startswith("_"): + cls_dict.update({ik: iv}) + out_dict.update({k: cls_dict}) + return out_dict def _clean_tuner_values(self, payload: Spockspace) -> Dict: # Just a double nested dict comprehension to unroll to dicts From fa87eb63931b68abe39a7b565c1741e0c7e7f17f Mon Sep 17 00:00:00 2001 From: Nicholas Cilfone <23509131+ncilfone@users.noreply.github.com> Date: Mon, 6 Mar 2023 14:56:11 -0500 Subject: [PATCH 06/14] updated docs --- website/docs/advanced_features/Post-Hooks.md | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/website/docs/advanced_features/Post-Hooks.md b/website/docs/advanced_features/Post-Hooks.md index 3bcb0a47..47453dad 100644 --- a/website/docs/advanced_features/Post-Hooks.md +++ b/website/docs/advanced_features/Post-Hooks.md @@ -62,19 +62,13 @@ class UploadMode(Enum): end_of_job = "EndOfJob" continuous = "Continuous" - @spock -class SMBaseOutputConfig: +class S3ScratchConfig: name: Optional[str] = None - source: str - destination: str + source: str = "s3://my-path" + destination: str = "/local/path" upload_mode: UploadMode = UploadMode.end_of_job - -@spock -class S3ScratchConfig(SMBaseOutputConfig): - ... - def __post_hook__(self): print(self.name) From 71bbd88a73818882daa958817de9c49a41bb0688 Mon Sep 17 00:00:00 2001 From: Nicholas Cilfone <23509131+ncilfone@users.noreply.github.com> Date: Mon, 6 Mar 2023 15:06:34 -0500 Subject: [PATCH 07/14] bumping ubuntu version to 20.04 -- this should still aloow for testing on python 3.6 --- .github/workflows/python-coverage.yaml | 2 +- .github/workflows/python-docs.yaml | 2 +- .github/workflows/python-lint.yaml | 2 +- .github/workflows/python-manual-docs.yaml | 2 +- .github/workflows/python-publish.yaml | 2 +- .github/workflows/python-pytest-s3.yaml | 2 +- .github/workflows/python-pytest-tune.yaml | 2 +- .github/workflows/python-pytest.yml | 2 +- .github/workflows/python-test-docs.yaml | 2 +- 9 files changed, 9 insertions(+), 9 deletions(-) diff --git a/.github/workflows/python-coverage.yaml b/.github/workflows/python-coverage.yaml index b50d8ee8..9a15a9a4 100644 --- a/.github/workflows/python-coverage.yaml +++ b/.github/workflows/python-coverage.yaml @@ -11,7 +11,7 @@ on: jobs: build: - runs-on: ubuntu-18.04 + runs-on: ubuntu-20.04 steps: - uses: actions/checkout@v2 diff --git a/.github/workflows/python-docs.yaml b/.github/workflows/python-docs.yaml index 5fcd99ad..6696b267 100644 --- a/.github/workflows/python-docs.yaml +++ b/.github/workflows/python-docs.yaml @@ -10,7 +10,7 @@ on: jobs: deploy: - runs-on: ubuntu-18.04 + runs-on: ubuntu-20.04 steps: - uses: actions/checkout@v2 diff --git a/.github/workflows/python-lint.yaml b/.github/workflows/python-lint.yaml index 94325eb3..3d524ac7 100644 --- a/.github/workflows/python-lint.yaml +++ b/.github/workflows/python-lint.yaml @@ -12,7 +12,7 @@ on: jobs: run_lint: - runs-on: ubuntu-18.04 + runs-on: ubuntu-20.04 steps: - uses: actions/checkout@v2 diff --git a/.github/workflows/python-manual-docs.yaml b/.github/workflows/python-manual-docs.yaml index 83f090fb..139d9976 100644 --- a/.github/workflows/python-manual-docs.yaml +++ b/.github/workflows/python-manual-docs.yaml @@ -7,7 +7,7 @@ on: workflow_dispatch jobs: deploy: - runs-on: ubuntu-18.04 + runs-on: ubuntu-20.04 steps: - uses: actions/checkout@v2 diff --git a/.github/workflows/python-publish.yaml b/.github/workflows/python-publish.yaml index c9776df1..8e0eca4d 100644 --- a/.github/workflows/python-publish.yaml +++ b/.github/workflows/python-publish.yaml @@ -11,7 +11,7 @@ on: jobs: deploy: - runs-on: ubuntu-18.04 + runs-on: ubuntu-20.04 steps: - uses: actions/checkout@v2 diff --git a/.github/workflows/python-pytest-s3.yaml b/.github/workflows/python-pytest-s3.yaml index 27961247..80d9d32c 100644 --- a/.github/workflows/python-pytest-s3.yaml +++ b/.github/workflows/python-pytest-s3.yaml @@ -11,7 +11,7 @@ on: jobs: build: - runs-on: ubuntu-18.04 + runs-on: ubuntu-20.04 strategy: matrix: python-version: ["3.6", "3.7", "3.8", "3.9", "3.10"] diff --git a/.github/workflows/python-pytest-tune.yaml b/.github/workflows/python-pytest-tune.yaml index 2f053ce6..f276211e 100644 --- a/.github/workflows/python-pytest-tune.yaml +++ b/.github/workflows/python-pytest-tune.yaml @@ -11,7 +11,7 @@ on: jobs: build: - runs-on: ubuntu-18.04 + runs-on: ubuntu-20.04 strategy: matrix: python-version: ["3.7", "3.8", "3.9", "3.10"] diff --git a/.github/workflows/python-pytest.yml b/.github/workflows/python-pytest.yml index b4a9976e..38bca867 100644 --- a/.github/workflows/python-pytest.yml +++ b/.github/workflows/python-pytest.yml @@ -11,7 +11,7 @@ on: jobs: build: - runs-on: ubuntu-18.04 + runs-on: ubuntu-20.04 strategy: matrix: python-version: ["3.6", "3.7", "3.8", "3.9", "3.10"] diff --git a/.github/workflows/python-test-docs.yaml b/.github/workflows/python-test-docs.yaml index 381736c7..75ed90ac 100644 --- a/.github/workflows/python-test-docs.yaml +++ b/.github/workflows/python-test-docs.yaml @@ -15,7 +15,7 @@ on: jobs: deploy: - runs-on: ubuntu-18.04 + runs-on: ubuntu-20.04 steps: - uses: actions/checkout@v2 From c9212ddeccae4bf0056b88275adbc0e62ac84782 Mon Sep 17 00:00:00 2001 From: Nicholas Cilfone <23509131+ncilfone@users.noreply.github.com> Date: Mon, 6 Mar 2023 15:41:57 -0500 Subject: [PATCH 08/14] remove old method --- spock/backend/wrappers.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/spock/backend/wrappers.py b/spock/backend/wrappers.py index ef628952..a5bd37f2 100644 --- a/spock/backend/wrappers.py +++ b/spock/backend/wrappers.py @@ -20,10 +20,6 @@ class Spockspace(argparse.Namespace): def __init__(self, **kwargs): super(Spockspace, self).__init__(**kwargs) - @property - def clean(self): - return Spockspace(**self.__repr_dict__) - @property def __repr_dict__(self): """Handles making a clean dict to hide the salt and key on print""" From b3d3973788d2c89a828d874485965262626d0642 Mon Sep 17 00:00:00 2001 From: Nicholas Cilfone <23509131+ncilfone@users.noreply.github.com> Date: Mon, 6 Mar 2023 15:46:55 -0500 Subject: [PATCH 09/14] fixed missing condition on if statement for hooks --- spock/backend/config.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spock/backend/config.py b/spock/backend/config.py index 484e4b52..c2fc27c2 100644 --- a/spock/backend/config.py +++ b/spock/backend/config.py @@ -160,7 +160,7 @@ def _handle_hooks( # Copy over the post init function -- borrow a bit from attrs library to add the # __post__hook__ method and/or the __maps__ method (via a shim method) to the init # call via `"__attrs_post_init__"` - if hasattr(cls, "__post_hook__") or hasattr(cls, "__maps__"): + if hasattr(cls, "__post_hook__") or hasattr(cls, "__maps__") or (len(hooks) > 0): # Force the post_hook function to have no explict return if hasattr(cls, "__post_hook__") and contains_return(cls.__post_hook__): raise _SpockInstantiationError( From b6197332006a66358196d472d89b970e3e8f0813 Mon Sep 17 00:00:00 2001 From: Nicholas Cilfone <23509131+ncilfone@users.noreply.github.com> Date: Mon, 6 Mar 2023 16:22:07 -0500 Subject: [PATCH 10/14] fixed bug in variable resolver -- was casting back to directory and file types -- easier to just cast back to the native underlying str type --- spock/backend/builder.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/spock/backend/builder.py b/spock/backend/builder.py index 30e1bdc9..9b271e18 100644 --- a/spock/backend/builder.py +++ b/spock/backend/builder.py @@ -238,9 +238,14 @@ def _cast_all_maps(cls, cls_fields: Dict, changed_vars: Set) -> None: """ for val in changed_vars: + # Make sure we cast directory and file types back to strings + if getattr(cls.__attrs_attrs__, val).type.__name__ in ("directory", "file"): + value_type = str + else: + value_type = getattr(cls.__attrs_attrs__, val).type cls_fields[val] = VarResolver._attempt_cast( maybe_env=cls_fields[val], - value_type=getattr(cls.__attrs_attrs__, val).type, + value_type=value_type, ref_value=val, ) From 49374e2e436997ca3ec90c2a38f261b2c0ee8bbe Mon Sep 17 00:00:00 2001 From: Nicholas Cilfone <23509131+ncilfone@users.noreply.github.com> Date: Thu, 23 Mar 2023 12:34:58 -0400 Subject: [PATCH 11/14] fixes for None results within parent hooks --- spock/backend/config.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/spock/backend/config.py b/spock/backend/config.py index c2fc27c2..61427d8a 100644 --- a/spock/backend/config.py +++ b/spock/backend/config.py @@ -181,10 +181,12 @@ def __shim__(self): cls.__post_hook__(self) # Call the parents hooks all_hooks = [val(self) for val in hooks] + # Pop any None values + all_hooks = [val for val in all_hooks if val is not None] # Add in the given hook if hasattr(cls, "__maps__"): all_hooks = [cls.__maps__(self)] + all_hooks - elif len(all_hooks) == 1: + if len(all_hooks) == 1: all_hooks = all_hooks[0] # Set maps to the mapped values object.__setattr__(self, "_maps", all_hooks) From b09bf5555004be65849d823e3300116ddb6babeb Mon Sep 17 00:00:00 2001 From: Nicholas Cilfone <23509131+ncilfone@users.noreply.github.com> Date: Tue, 25 Jul 2023 15:43:00 -0400 Subject: [PATCH 12/14] removed __maps__ from repr for print --- spock/backend/wrappers.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/spock/backend/wrappers.py b/spock/backend/wrappers.py index a5bd37f2..aef3b9fc 100644 --- a/spock/backend/wrappers.py +++ b/spock/backend/wrappers.py @@ -24,7 +24,9 @@ def __init__(self, **kwargs): def __repr_dict__(self): """Handles making a clean dict to hide the salt and key on print""" clean_dict = { - k: v for k, v in self.__dict__.items() if k not in {"__key__", "__salt__"} + k: v + for k, v in self.__dict__.items() + if k not in {"__key__", "__salt__", "__maps__"} } repr_dict = {} for k, v in clean_dict.items(): From 9d20f308b96d2efa024892cee5168f717c797ee0 Mon Sep 17 00:00:00 2001 From: Nicholas Cilfone <23509131+ncilfone@users.noreply.github.com> Date: Tue, 24 Oct 2023 17:28:51 -0400 Subject: [PATCH 13/14] Update validators.py Force change to not check read write privs Signed-off-by: Nicholas Cilfone <23509131+ncilfone@users.noreply.github.com> --- spock/backend/validators.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spock/backend/validators.py b/spock/backend/validators.py index 4f86c3c2..255ac186 100644 --- a/spock/backend/validators.py +++ b/spock/backend/validators.py @@ -261,7 +261,7 @@ def __call__(self, inst: _C, attr: attr.Attribute, value: Any) -> None: and self.type[0].__name__ == "directory" ): return _is_directory( - self.type, create=True, check_access=True, attr=attr, value=value + self.type, create=True, check_access=False, attr=attr, value=value ) # Catch the file type -- tuples suck, so we need to handle them with their own # condition here -- basically if the tuple is of type directory then we need @@ -271,7 +271,7 @@ def __call__(self, inst: _C, attr: attr.Attribute, value: Any) -> None: and hasattr(self.type[0], "__name__") and self.type[0].__name__ == "file" ): - return _is_file(type=self.type, check_access=True, attr=attr, value=value) + return _is_file(type=self.type, check_access=False, attr=attr, value=value) # Fallback on base attr else: return _check_instance(value=value, name=attr.name, type=self.type) From 568419241bcfd6f058b266105c32d7c195a7576d Mon Sep 17 00:00:00 2001 From: Nicholas Cilfone <23509131+ncilfone@users.noreply.github.com> Date: Fri, 3 Nov 2023 10:07:33 -0400 Subject: [PATCH 14/14] Update test_type_specific.py Removed permissions tests as it now defaults to false on checking r+w since this can break things on AWS Signed-off-by: Nicholas Cilfone <23509131+ncilfone@users.noreply.github.com> --- tests/base/test_type_specific.py | 200 +++++++++++++++---------------- 1 file changed, 100 insertions(+), 100 deletions(-) diff --git a/tests/base/test_type_specific.py b/tests/base/test_type_specific.py index 44844836..84d5d28d 100644 --- a/tests/base/test_type_specific.py +++ b/tests/base/test_type_specific.py @@ -192,103 +192,103 @@ class FileFail: config.generate() -class TestWrongPermission: - def test_dir_write_permission(self, monkeypatch, tmp_path): - """Tests directory write permission check""" - with monkeypatch.context() as m: - m.setattr( - sys, - "argv", - [""], - ) - dir = f"{str(tmp_path)}/fail_perms" - os.mkdir(dir) - subprocess.run(["chmod", "444", dir]) - - with pytest.raises(_SpockInstantiationError): - - @spock - class DirWrongPermissions: - test_dir: directory = dir - - config = ConfigArgBuilder(DirWrongPermissions, desc="Test Builder") - config.generate() - subprocess.run(["chmod", "777", dir]) - os.rmdir(dir) - - def test_dir_read_permission(self, monkeypatch, tmp_path): - """Tests directory read permission check""" - with monkeypatch.context() as m: - m.setattr( - sys, - "argv", - [""], - ) - dir = f"{str(tmp_path)}/fail_perms" - os.mkdir(dir) - subprocess.run(["chmod", "222", dir]) - - with pytest.raises(_SpockInstantiationError): - - @spock - class DirWrongPermissions: - test_dir: directory = dir - - config = ConfigArgBuilder(DirWrongPermissions, desc="Test Builder") - config.generate() - subprocess.run(["chmod", "777", dir]) - os.rmdir(dir) - - def test_file_write_permission(self, monkeypatch, tmp_path): - """Tests file write permission check""" - with monkeypatch.context() as m: - m.setattr( - sys, - "argv", - [""], - ) - - dir = f"{str(tmp_path)}/fail_perms" - os.mkdir(dir) - f = open(f"{dir}/tmp_fail.txt", "x") - f.close() - - subprocess.run(["chmod", "444", f"{dir}/tmp_fail.txt"]) - - with pytest.raises(_SpockInstantiationError): - - @spock - class FileWrongPermissions: - test_file: file = f"{dir}/tmp_fail.txt" - - config = ConfigArgBuilder(FileWrongPermissions, desc="Test Builder") - config.generate() - subprocess.run(["chmod", "777", f"{dir}/tmp_fail.txt"]) - os.remove(f"{dir}/tmp_fail.txt") - - def test_file_read_permission(self, monkeypatch, tmp_path): - """Tests file read permission check""" - with monkeypatch.context() as m: - m.setattr( - sys, - "argv", - [""], - ) - - dir = f"{str(tmp_path)}/fail_perms" - os.mkdir(dir) - f = open(f"{dir}/tmp_fail.txt", "x") - f.close() - - subprocess.run(["chmod", "222", f"{dir}/tmp_fail.txt"]) - - with pytest.raises(_SpockInstantiationError): - - @spock - class FileWrongPermissions: - test_file: file = f"{dir}/tmp_fail.txt" - - config = ConfigArgBuilder(FileWrongPermissions, desc="Test Builder") - config.generate() - subprocess.run(["chmod", "777", f"{dir}/tmp_fail.txt"]) - os.remove(f"{dir}/tmp_fail.txt") +# class TestWrongPermission: +# def test_dir_write_permission(self, monkeypatch, tmp_path): +# """Tests directory write permission check""" +# with monkeypatch.context() as m: +# m.setattr( +# sys, +# "argv", +# [""], +# ) +# dir = f"{str(tmp_path)}/fail_perms" +# os.mkdir(dir) +# subprocess.run(["chmod", "444", dir]) + +# with pytest.raises(_SpockInstantiationError): + +# @spock +# class DirWrongPermissions: +# test_dir: directory = dir + +# config = ConfigArgBuilder(DirWrongPermissions, desc="Test Builder") +# config.generate() +# subprocess.run(["chmod", "777", dir]) +# os.rmdir(dir) + +# def test_dir_read_permission(self, monkeypatch, tmp_path): +# """Tests directory read permission check""" +# with monkeypatch.context() as m: +# m.setattr( +# sys, +# "argv", +# [""], +# ) +# dir = f"{str(tmp_path)}/fail_perms" +# os.mkdir(dir) +# subprocess.run(["chmod", "222", dir]) + +# with pytest.raises(_SpockInstantiationError): + +# @spock +# class DirWrongPermissions: +# test_dir: directory = dir + +# config = ConfigArgBuilder(DirWrongPermissions, desc="Test Builder") +# config.generate() +# subprocess.run(["chmod", "777", dir]) +# os.rmdir(dir) + +# def test_file_write_permission(self, monkeypatch, tmp_path): +# """Tests file write permission check""" +# with monkeypatch.context() as m: +# m.setattr( +# sys, +# "argv", +# [""], +# ) + +# dir = f"{str(tmp_path)}/fail_perms" +# os.mkdir(dir) +# f = open(f"{dir}/tmp_fail.txt", "x") +# f.close() + +# subprocess.run(["chmod", "444", f"{dir}/tmp_fail.txt"]) + +# with pytest.raises(_SpockInstantiationError): + +# @spock +# class FileWrongPermissions: +# test_file: file = f"{dir}/tmp_fail.txt" + +# config = ConfigArgBuilder(FileWrongPermissions, desc="Test Builder") +# config.generate() +# subprocess.run(["chmod", "777", f"{dir}/tmp_fail.txt"]) +# os.remove(f"{dir}/tmp_fail.txt") + +# def test_file_read_permission(self, monkeypatch, tmp_path): +# """Tests file read permission check""" +# with monkeypatch.context() as m: +# m.setattr( +# sys, +# "argv", +# [""], +# ) + +# dir = f"{str(tmp_path)}/fail_perms" +# os.mkdir(dir) +# f = open(f"{dir}/tmp_fail.txt", "x") +# f.close() + +# subprocess.run(["chmod", "222", f"{dir}/tmp_fail.txt"]) + +# with pytest.raises(_SpockInstantiationError): + +# @spock +# class FileWrongPermissions: +# test_file: file = f"{dir}/tmp_fail.txt" + +# config = ConfigArgBuilder(FileWrongPermissions, desc="Test Builder") +# config.generate() +# subprocess.run(["chmod", "777", f"{dir}/tmp_fail.txt"]) +# os.remove(f"{dir}/tmp_fail.txt")