chore: raise an error if the engine does not support a particular feature #88
Conversation
product-auto-label
bot
added
size: m
| bpd.options.bigquery.credentials = context.credentials | ||
| raise ValueError( | ||
| "Please install the 'bigframes' package (pip install bigframes) to use the bigframes engine." | ||
| ) |
There was a problem hiding this comment.
Let's keep this intact for now.
| ) | |
| ) | |
| bpd.options.bigquery.project = context.project | |
| bpd.options.bigquery.credentials = context.credentials |
At some point we will want to refactor this logic to use the new magics_settings, but not in this PR.
| # This missing reason is for options that are magics-only and apply to all | ||
| # engines. For example, the destination variable is a magics-only setting | ||
| # and doesn't affect how queries are handled by the engine. | ||
| ENGINE_UNIVERSAL = enum.auto() |
There was a problem hiding this comment.
Per our discussion offline about the spreadsheet, let's add an additional option for SUPPORTED_BY_ENGINE_BUT_NO_OPTION for things where there's some support for it in the engine (e.g. by setting the cell_arg), but there's no global option to override it.
| ENGINE_UNIVERSAL = enum.auto() | |
| ENGINE_UNIVERSAL = enum.auto() | |
| # Similar to ENGINE_UNIVERSAL, these options _do_ work, but there | |
| # isn't a way to globally override them. Use this instead of ENGINE_UNIVERSAL | |
| # for things that really do require some explicit support in the engine (e.g. query parameters) | |
| # but don't have a way to set them for all queries. | |
| SUPPORTED_BY_ENGINE_BUT_NO_OPTION = enum.auto() |
bigquery_magics/config.py
Outdated
| description="Limits the number of rows in the returned DataFrame.", | ||
| cell_arg="--max_results", | ||
| magics_context=MissingReason.NOT_SUPPORTED_BY_ENGINE_BUT_POSSIBLE, | ||
| bigframes_option="read_gbq_query.max_results" |
There was a problem hiding this comment.
SUPPORTED_BY_ENGINE_BUT_NO_OPTION
Anything on read_gbq_query is not a global option and we shouldn't include it here or in our docs for the magic. That's just an implementation detail.
bigquery_magics/config.py
Outdated
| MagicsSetting( | ||
| description="Set it to a query to estimate costs.", | ||
| cell_arg="--dry_run", | ||
| magics_context=MissingReason.NOT_SUPPORTED_BY_ENGINE_BUT_POSSIBLE, |
There was a problem hiding this comment.
SUPPORTED_BY_ENGINE_BUT_NO_OPTION
bigquery_magics/config.py
Outdated
| description="Set it to a query to estimate costs.", | ||
| cell_arg="--dry_run", | ||
| magics_context=MissingReason.NOT_SUPPORTED_BY_ENGINE_BUT_POSSIBLE, | ||
| bigframes_option="read_gbq_query" |
There was a problem hiding this comment.
NOT_SUPPORTED_BY_ENGINE_BUT_POSSIBLE
This one we haven't implemented yet. (Or maybe we have via the configuration argument in read_gbq_query? If so, then it's SUPPORTED_BY_ENGINE_BUT_NO_OPTION)
bigquery_magics/config.py
Outdated
| MagicsSetting( | ||
| description="Set it to use instead of Standard SQL.", | ||
| cell_arg="--use_legacy_sql", | ||
| magics_context=MissingReason.NOT_SUPPORTED_BY_ENGINE_BUT_POSSIBLE, |
There was a problem hiding this comment.
We will almost never have NOT_SUPPORTED_BY_ENGINE_BUT_POSSIBLE for magics_context because that was the first engine we created (pandas engine). It's the bigframes one that is missing features like this.
If there is something missing, SUPPORTED_BY_ENGINE_BUT_NO_OPTION is likely the reason.
Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:
Fixes #384564457 🦕