-
Notifications
You must be signed in to change notification settings - Fork 21
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
Reading custom endpoint results SDK support #132
Conversation
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.
Looks good so far. Looking forward to some tests!
Co-authored-by: Benjamin Smith <[email protected]>
…client into add-custom-endpoints
@@ -12,7 +12,7 @@ | |||
ResultsResponse, | |||
) | |||
|
|||
|
|||
# pylint: disable=duplicate-code |
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.
This is likely because of all the filter arguments and suggests a deeper change to the project. Introduce a ResultFilter
structure that contains all the filtering possibilities:
@dataclass
class ResultFilter:
limit: Optional[int] = None
offset: Optional[int] = None
columns: Optional[List[str]] = None
sample_count: Optional[int] = None
filters: Optional[str] = None
sort_by: Optional[List[str]] = None
Then here would look more like:
def get_custom_endpoint_result(
self,
handle: str,
endpoint: str,
filter: Optional[ResultFilter]
) -> ResultsResponse:
It would resolve both duplicated code lint warning and also reduce the "argument overload" lint error that is being globally suppressed here
Line 4 in ed3836d
max-args=10 |
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.
I will log this and do at another time.
This allows for fetching custom-endpoint results. example:
Todo: add some testing