uranusjr commented on a change in pull request #20019:
URL: https://github.com/apache/airflow/pull/20019#discussion_r763639901
##########
File path: airflow/models/param.py
##########
@@ -154,30 +170,34 @@ def __getitem__(self, key: str) -> Any:
:param key: The key to fetch
:type key: str
"""
- param = super().__getitem__(key)
+ param = self.__dict[key]
return param.resolve(suppress_exception=self.suppress_exception)
+ def get_param(self, key: str) -> Param:
+ """Get the internal :class:`.Param` object for this key"""
+ return self.__dict[key]
+
+ def items(self):
+ return ItemsView(self.__dict)
+
+ def values(self):
+ return ValuesView(self.__dict)
+
+ def update(self, *args, **kwargs) -> None:
+
+ if len(args) == 1 and isinstance(args[0], ParamsDict):
Review comment:
```suggestion
if len(args) == 1 and not kwargs and isinstance(args[0], ParamsDict):
```
If this is called with an invalid argument combination like
`params.update({"b": 1}, c=2)`, we want to let the `super()` call catch it and
emit an error.
##########
File path: tests/models/test_param.py
##########
@@ -167,18 +167,28 @@ def test_params_dict(self):
assert pd3.dump() == {'key': 'value'}
# Validate the ParamsDict
- pd.validate()
+ plain_dict = pd.validate()
+ assert type(plain_dict) == dict
pd2.validate()
pd3.validate()
# Update the ParamsDict
- with pytest.raises(ValueError):
+ with pytest.raises(ValueError, match=r'Invalid input for param key: 1
is not'):
Review comment:
I assume this is not the complete error message? (If it is, we should
change it…) It’s probably a good idea to add an extra assert to make sure the
full error message is as expected.
##########
File path: airflow/models/param.py
##########
@@ -121,9 +122,21 @@ def __init__(self, dict_obj: Optional[Dict] = None,
suppress_exception: bool = F
params_dict[k] = Param(v)
else:
params_dict[k] = v
- super().__init__(params_dict)
+ self.__dict = params_dict
self.suppress_exception = suppress_exception
+ def __contains__(self, o: object) -> bool:
+ return o in self.__dict
+
+ def __len__(self) -> int:
+ return len(self.__dict)
+
+ def __delitem__(self, v: str) -> None:
+ del self.__dict[v]
+
+ def __iter__(self):
+ return self.__dict.__iter__()
Review comment:
```suggestion
return iter(self.__dict)
```
For consistency—we didn’t use the dict’s magic methods for all other magic
method implementations.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
