Skip to content

Commit

Permalink
Merge pull request #1462 from PrefectHQ/protect-dotdict
Browse files Browse the repository at this point in the history
Remove restriction on DotDict critical keys
  • Loading branch information
jlowin authored Sep 7, 2019
2 parents ecf9a93 + d616f57 commit 9701a91
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 58 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ These changes are available in the [master branch](https://github.com/PrefectHQ/
- Add `--base-url` argument for Docker daemons to `agent start` CLI command - [#1441](https://github.com/PrefectHQ/prefect/pull/1441)
- Add environment labels for organizing / tagging different Flow execution environments - [#1438](https://github.com/PrefectHQ/prefect/issues/1438)
- Use `-U` option when installing `prefect` in Docker containers to override base image version - [#1461](https://github.com/PrefectHQ/prefect/pull/1461)
- Remove restriction that prevented `DotDict` classes from having keys that shadowed dict methods - [#1462](https://github.com/PrefectHQ/prefect/pull/1462)

### Task Library

Expand All @@ -23,6 +24,7 @@ These changes are available in the [master branch](https://github.com/PrefectHQ/

- Fix incorrect import in `DaskKubernetesEnvironment` job template - [#1458](https://github.com/PrefectHQ/prefect/pull/1458)
- Raise error on Agents started without an appropriate API token - [#1459](https://github.com/PrefectHQ/prefect/pull/1459)
- Fix bug when calling `as_nested_dict` on `DotDicts` with an `items` key - [#1462](https://github.com/PrefectHQ/prefect/pull/1462)

### Deprecations

Expand Down
27 changes: 11 additions & 16 deletions src/prefect/utilities/collections.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,12 +54,11 @@ class DotDict(MutableMapping):
```
"""

__protect_critical_keys__ = True

def __init__(self, init_dict: DictLike = None, **kwargs: Any):
# a DotDict could have a key that shadows `update`
if init_dict:
self.update(init_dict)
self.update(kwargs)
super().update(init_dict)
super().update(kwargs)

def get(self, key: str, default: Any = None) -> Any:
"""
Expand All @@ -79,13 +78,6 @@ def __getitem__(self, key: str) -> Any:
return self.__dict__[key] # __dict__ expects string keys

def __setitem__(self, key: str, value: Any) -> None:
# prevent overwriting any critical attributes
if (
self.__protect_critical_keys__
and isinstance(key, str)
and hasattr(MutableMapping, key)
):
raise ValueError('Invalid key: "{}"'.format(key))
self.__dict__[key] = value

def __setattr__(self, attr: str, value: Any) -> None:
Expand Down Expand Up @@ -167,11 +159,14 @@ def as_nested_dict(
if isinstance(obj, (list, tuple, set)):
return type(obj)([as_nested_dict(d, dct_class) for d in obj])
elif isinstance(obj, (dict, DotDict)):
# instantiate the dict and call update because if a dotdict contains a key called
# `update`, then calling update in __init__ becomes impossible
new_dict = dct_class()
new_dict.update({k: as_nested_dict(v, dct_class) for k, v in obj.items()})
return new_dict
# DotDicts could have keys that shadow `update` and `items`, so we
# take care to avoid accessing those keys here
return dct_class(
{
k: as_nested_dict(v, dct_class)
for k, v in getattr(obj, "__dict__", obj).items()
}
)
return obj


Expand Down
11 changes: 5 additions & 6 deletions src/prefect/utilities/graphql.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,12 @@ def lowercase_first_letter(s: str) -> str:


class GraphQLResult(DotDict):
__protect_critical_keys__ = False

def __repr__(self) -> str:
self_as_dict = as_nested_dict(self, dct_class=dict)
try:
return json.dumps(as_nested_dict(self, dict), indent=4)
return json.dumps(self_as_dict, indent=4)
except TypeError:
return repr(self.to_dict())
return repr(self_as_dict)


class EnumValue:
Expand Down Expand Up @@ -282,7 +281,7 @@ def with_args(field: Any, arguments: Any) -> str:
def compress(input: Any) -> str:
"""
Convenience function for compressing something before sending
it to Cloud. Converts to string, encodes, compresses,
it to Cloud. Converts to string, encodes, compresses,
encodes again using b64, and decodes.
Args:
Expand All @@ -297,7 +296,7 @@ def compress(input: Any) -> str:
def decompress(string: str) -> Any:
"""
Convenience function for decompressing a string that's been
compressed. Base64 decodes the string, decodes it,
compressed. Base64 decodes the string, decodes it,
decompresses it, and loads.
Args:
Expand Down
45 changes: 9 additions & 36 deletions tests/utilities/test_collections.py
Original file line number Diff line number Diff line change
Expand Up @@ -114,17 +114,6 @@ def test_len(self):
del d["chris"]
assert len(d) == 2

@pytest.mark.parametrize("key", ["keys", "update", "get", "items"])
def test_reserved_attrs_raise_error_on_init(self, key):
with pytest.raises(ValueError):
d = DotDict({key: 5})

@pytest.mark.parametrize("key", ["keys", "update", "get", "items"])
def test_reserved_attrs_raise_error_on_set(self, key):
with pytest.raises(ValueError):
d = DotDict(data=5)
d.__setattr__(key, "value")

def test_attr_updates_and_key_updates_agree(self):
d = DotDict(data=5)
d.data += 1
Expand Down Expand Up @@ -178,6 +167,10 @@ def test_repr_sorts_mixed_keys(self):
d["b"] = 1
assert repr(d) == "<DotDict: 'a', 'b', 1>"

def test_copy(self):
d = DotDict(a=1, b=2, c=3)
assert d == d.copy()

def test_eq_empty(self):
assert DotDict() == DotDict()

Expand Down Expand Up @@ -306,31 +299,11 @@ def test_merge_nested_dicts_with_empty_section(dct_class):
assert merge_dicts(b, a) == a


def test_protect_critical_default_true():
x = DotDict()
assert x.__protect_critical_keys__


def test_protect_critical_keys_active():
x = DotDict()
with pytest.raises(ValueError):
x.update = 1


def test_protect_critical_default_false_for_graphql_result():
x = GraphQLResult()
assert not x.__protect_critical_keys__


def test_protect_critical_keys_inactive_for_graphql_result():
x = GraphQLResult()
x.update = 1
assert x.update == 1


def test_protect_critical_keys_inactive_for_graphql_result_init():
x = GraphQLResult(update=1)
assert x.update == 1
def test_as_nested_dict_works_when_critical_keys_shadowed():
x = dict(update=1, items=2)
y = as_nested_dict(x, DotDict)
assert y.update == 1
assert y.items == 2


def test_protect_critical_keys_inactive_for_nested_query():
Expand Down

0 comments on commit 9701a91

Please sign in to comment.