Better value validation for state parameter of fetch_*_table
#1616
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## dev/fetch-ui-parity #1616 +/- ##
=======================================================
- Coverage 76.54% 76.54% -0.01%
=======================================================
Files 292 292
Lines 14671 14678 +7
=======================================================
+ Hits 11230 11235 +5
- Misses 3441 3443 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
| @@ -80,6 +81,11 @@ def verify_type(var_name: str, var, expected_type: Union[type, tuple]): | |||
| raise TypeError("{} is a stream, which does not implement read method".format(var_name)) | |||
|
|
|||
|
|
|||
| def verify_value(var_name: str, var, expected_values: Iterable[T]): | |||
| @@ -189,7 +192,7 @@ def fetch_runs_table( | |||
| self, | |||
| *, | |||
| id: Optional[Union[str, Iterable[str]]] = None, | |||
| state: Optional[Union[str, Iterable[str]]] = None, | |||
| state: Optional[Union[Literal["inactive", "active"], Iterable[Literal['inactive", "active']]]] = None, | |||
There was a problem hiding this comment.
Don't understand ' vs " order here
There was a problem hiding this comment.
Me either 😉. Fixed 🚀
| @@ -286,6 +289,10 @@ def fetch_runs_table( | |||
| verify_type("sort_by", sort_by, str) | |||
| verify_type("ascending", ascending, bool) | |||
|
|
|||
| if states is not None: | |||
There was a problem hiding this comment.
From what I see, states will never be None, right? The type hint for the as_list function doesn't seem to be adequate, or am I missing something?
There was a problem hiding this comment.
I know that the as_list function is not part of your PR, but just noticed that its confusing return type hint requires additional, unnecessary if statement
There was a problem hiding this comment.
Thanks a lot! Fixed I think
Before submitting checklist