-
Notifications
You must be signed in to change notification settings - Fork 14.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[AIRFLOW-5819] Update AWSBatchOperator default value #6473
Conversation
Codecov Report
@@ Coverage Diff @@
## master #6473 +/- ##
==========================================
- Coverage 83.82% 83.62% -0.21%
==========================================
Files 635 635
Lines 36657 36716 +59
==========================================
- Hits 30727 30702 -25
- Misses 5930 6014 +84
Continue to review full report at Codecov.
|
@@ -83,7 +83,7 @@ class AWSBatchOperator(BaseOperator): | |||
template_fields = ('job_name', 'overrides',) | |||
|
|||
@apply_defaults | |||
def __init__(self, job_name, job_definition, job_queue, overrides, array_properties=None, | |||
def __init__(self, job_name, job_definition, job_queue, overrides, array_properties={}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You shouldn't do it like this. https://docs.python-guide.org/writing/gotchas/#mutable-default-arguments
Instead in the method do
self.array_properties = array_properties if array_properties is not None else {}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for pointing out, updated.
a76d312
to
b3b18a4
Compare
Can you fix the failing tests too:
|
The default value of `None` throws an error and should be changed to `{}`
b3b18a4
to
1ccd24d
Compare
Co-Authored-By: Felix Uellendall <[email protected]>
(cherry picked from commit 63ba1c5)
(cherry picked from commit 63ba1c5)
(cherry picked from commit 63ba1c5)
Make sure you have checked all steps below.
Jira
Description
The default value
None
throws an error, should be changed to{}
.Tests
I am actually not sure why the issue was not captured in the existing test:
airflow/tests/contrib/operators/test_awsbatch_operator.py
Line 39 in 73bf718
Commits
Documentation