potiuk commented on a change in pull request #10498:
URL: https://github.com/apache/airflow/pull/10498#discussion_r475295182
##########
File path: airflow/www/views.py
##########
@@ -2329,28 +2411,29 @@ def process_form(self, form, is_created):
form.extra.data = json.dumps(extra)
def prefill_form(self, form, pk):
+ """Prefill the form."""
try:
- d = json.loads(form.data.get('extra', '{}'))
+ extra_dictionary = json.loads(form.data.get('extra', '{}'))
except JSONDecodeError:
- d = {}
+ extra_dictionary = {}
- if not hasattr(d, 'get'):
- logging.warning('extra field for {} is not iterable'.format(
- form.data.get('conn_id', '<unknown>')))
+ if not hasattr(extra_dictionary, 'get'):
Review comment:
Could be - but I prefered to have minimal number of actual logic change
in this PR and focus on refactoring removing pylint warnings without impacting
the logic too much as it would be a bit risky. I'd prefer to leave it as-is.
There might be some cases where this object is different than a dict, so i
prefer to leave it as is. I think the right time to change it is when we add
mypy typing - then we will be more certain of the types of each file, but IMHO
that should be a different change. I would love to keep it only focused on
naming and other stuff which is pylint-related.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]