XD-DENG commented on a change in pull request #4911: [AIRFLOW-3768] Escape
search parameter in pagination controls
URL: https://github.com/apache/airflow/pull/4911#discussion_r265832553
##########
File path: airflow/www/utils.py
##########
@@ -68,17 +69,11 @@ def should_hide_value_for_key(key_name):
def get_params(**kwargs):
- params = []
- for k, v in kwargs.items():
- if k == 'showPaused':
- # True is default or None
- if v or v is None:
- continue
- params.append('{}={}'.format(k, v))
- elif v:
- params.append('{}={}'.format(k, v))
- params = sorted(params, key=lambda x: x.split('=')[0])
- return '&'.join(params)
+ if 'showPaused' in kwargs:
+ v = kwargs['showPaused']
+ if v or v is None:
+ kwargs.pop('showPaused')
+ return urlencode(kwargs)
Review comment:
Hi @ashb
There is a difference in the result. Let's say `kwargs` is `{'a':"100",
"c":None}`:
Result from previous implementation: `'a=100'`
Result from your new implementation: `'a=100&c=None'`
(Actually there is a potential bug in the previous implementation, let's say
my kwargs is `{'a':"100", "c":0}`. I would except `'a=100&c=0'` while it will
return `'a=100'`. It will be fixed by your change though)
Coming back to your change: I would suggest to add one more check to check
if value is `None`: if yes, pop out that key-value pair as well.
----------------------------------------------------------------
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]
With regards,
Apache Git Services
