Fix issue where empty WebUI property is not parsed correctly #3012
Conversation
| @@ -285,7 +285,7 @@ def swarm() -> Response: | |||
| if isinstance(parsed_options_value, bool): | |||
| parsed_options_dict[key] = value == "true" | |||
| elif parsed_options_value is None: | |||
There was a problem hiding this comment.
Maybe we can do:
elif parsed_options_value is None and value is None:
parsed_options_dict[key] = None
This way we still get the type casting from the else statement
There was a problem hiding this comment.
When the parsed_options_value is None, type checking does not make sense. I modified the code to catch this case.
There was a problem hiding this comment.
Type checking indeed doesn't make sense if the value is None, but take for example if you have an argument with a default value of None:
parser.add_argument('--number', default=None)
Here the parsed_options_value will always be None, however in some cases the value will be a number, in which case keeping the type may be useful
That's why I suggested elif parsed_options_value is None and value is None
|
Hey @timhovius! Thanks for reporting and fixing this issue! If you have time, it would be great if you could also add a test case for this scenario? |
|
Hi @timhovius ! Have you had any chance to look at making a test for this? |
Yes I'm looking into this currently. |
|
Actually it goes wrong when no default value was set. So I modified the tests to test this behavior and I added an extra test. |
| data={"user_count": 1, "spawn_rate": 1, "host": "", "my_argument": "42"}, | ||
| ) | ||
| self.assertEqual(200, response.status_code) | ||
| self.assertEqual(42, my_dict["val"]) |
There was a problem hiding this comment.
Nice! Thanks for adding tests!
This resolves #3011