dataprep icon indicating copy to clipboard operation
dataprep copied to clipboard

DataConnector should throw correct error if universal parameters are used but not defined

Open dovahcrow opened this issue 4 years ago • 2 comments

Describe the bug Currently, when there are no definitions for the universal parameters but is used in the query, it throws an assertion error instead of something more informative.

To Reproduce For example, this piece of code gives you an error.

dc.query("videos", q="covid", part="snippet", _count=100)

which is because, for youtube/videos, we don't have pagination implemented. But it gives you an assertion error. image.png

Expected behavior Clearer error message.

dovahcrow avatar Jul 08 '20 00:07 dovahcrow

Looks like this issue is now already taken care of in connector.py:

connector_pag

connector_search

Although, the behavior is not consistent across all universal parameters:

  1. When _count parameter is used and pagination is specified, it is ignored with a warning: pag_error

  2. When _q is used when search is not specified, an exception with an appropriate error message is thrown: search_err

@dovahcrow and @peiwangdb, this looks like the right thing to do. Please let me know if this looks okay or if anything needs to be further worked upon.

pallavibharadwaj avatar Oct 07 '20 07:10 pallavibharadwaj

Below are my thoughts on handling those errors. First, for both cases, in the error description, I wonder if we should tell the user explicitly that the universal parameters should be defined in the config file. For the _q case, throwing an error looks good since there is no other way to proceed. For _count, the possible cases:

  1. no _count definition, (works the above way, and _count is ignored)
  2. _count is smaller than a single page result number (r), (no problem)
  3. _count works properly, between r and r*max_page (no problem)
  4. _count is larger than r*max_page. (only the retrieved results will be returned. should we also add a prompt to let the user know the number of returned results is smaller than expected?)

@pallavibharadwaj could you double-check the code logic to make sure the execution is aligned with our expectation? And test and think about if there are other error cases we missed for those two parameters.

Thanks!

peiwangdb avatar Oct 07 '20 16:10 peiwangdb