BasPH commented on a change in pull request #20573:
URL: https://github.com/apache/airflow/pull/20573#discussion_r776637138
##########
File path: tests/utils/test_helpers.py
##########
@@ -262,3 +263,27 @@ def assert_exactly_one(true=0, truthy=0, false=0, falsy=0):
def test_exactly_one_should_fail(self):
with pytest.raises(ValueError):
exactly_one([True, False])
+
+ @pytest.mark.parametrize('mode', ['strict', 'truthy'])
+ def test_prune_dict(self, mode):
+ l1 = ['', 0, '1', None]
+ d1 = {'a': None, 'b': '', 'c': 'hi', 'd': l1}
+ d2 = {'a': None, 'b': '', 'c': d1, 'd': l1, 'e': [None, '', 0, d1, l1,
['']]}
+ str(d2)
Review comment:
This line isn't necessary?
##########
File path: airflow/providers/cncf/kubernetes/operators/kubernetes_pod.py
##########
@@ -597,3 +607,57 @@ def __exit__(self, exctype, excinst, exctb):
logger = logging.getLogger()
logger.error(str(excinst), exc_info=True)
return caught_error
+
+
+def _prune_dict(val: Any, mode='strict'):
+ """
+ Note: this is duplicated from ``airflow.utils.helpers.prune_dict``. That
one is tested
Review comment:
Could you clarify this message? I'm struggling to understand why it's
needed. In which version would you plan to remove it again?
##########
File path: airflow/providers/cncf/kubernetes/operators/kubernetes_pod.py
##########
@@ -573,6 +573,16 @@ def build_pod_request_obj(self, context=None):
)
return pod
+ def dry_run(self) -> None:
+ """
+ Prints out the pod definition that would be created by this operator
+ Does not include labels specific to the task instance (since there
isn't
+ one in a dry_run) and excludes all empty elements.
+ """
+ print("dry run:")
Review comment:
Would remove this line, will result in errors when people copy-paste the
output and don't know about this one line.
##########
File path: tests/utils/test_helpers.py
##########
@@ -262,3 +263,27 @@ def assert_exactly_one(true=0, truthy=0, false=0, falsy=0):
def test_exactly_one_should_fail(self):
with pytest.raises(ValueError):
exactly_one([True, False])
+
+ @pytest.mark.parametrize('mode', ['strict', 'truthy'])
+ def test_prune_dict(self, mode):
Review comment:
The code in this test reads odd since you have one half configured via
`@parameterize` and the other half in the if/else condition. Could you separate
it into two tests, or add the expected dict to the `@parameterize` arguments so
that you don't need the if/else condition in the test?
##########
File path: airflow/providers/cncf/kubernetes/operators/kubernetes_pod.py
##########
@@ -573,6 +573,16 @@ def build_pod_request_obj(self, context=None):
)
return pod
+ def dry_run(self) -> None:
+ """
+ Prints out the pod definition that would be created by this operator
Review comment:
```suggestion
Prints out the pod definition that would be created by this operator.
```
--
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]
