[PF-2195] Switch default BQ resolve delimiter and add a FULL_PATH_SQL option.#351
[PF-2195] Switch default BQ resolve delimiter and add a FULL_PATH_SQL option.#351
Conversation
gjuggler
changed the title
gjuggler
changed the title
melissachang
left a comment
melissachang
left a comment
There was a problem hiding this comment.
I'm not sure which should be default. It could be that a user rarely runs bq, and often runs SQL queries (eg in a notebook) against data collection BQ resources. If you want, I'm happy to shoot an email to solutions. The email could kill 2 birds with 1 stone: ask for their preference, and possibly notify them of breaking change.
If we keep bq as default, I can update UI to match. (In the future, this logic should be in FrontendServer.)
| /** This enum specifies the possible ways to resolve a BigQuery resource. */ | ||
| public enum BqResolvedOptions { | ||
| FULL_PATH, // For data table: [project id].[dataset id].[data table id if applicable] | ||
| FULL_PATH, // For data table: [project id]:[dataset id].[data table id if applicable] |
There was a problem hiding this comment.
Suggest FULL_PATH -> FULL_PATH_BQ and some comment changes
// project:dataset.table, used for bq CLI
FULL_PATH_BQ,
// project.dataset.table, used for SQL
FULL_PATH_SQL,
(I left out if applicable for brevity..)
|
|
||
| /** Gets cloud identifier for Dataset in full-path: [project id].[dataset id] */ | ||
| /** Gets cloud identifier for Dataset in full-path: [project id]:[dataset id] */ | ||
| public static String getDatasetFullPath(String projectId, String datasetId) { |
There was a problem hiding this comment.
getDatasetFullPathBq ?
👍 I emailed the solutions team, and will come back to this PR once we have a clearer signal on which default to prefer. |
2 participants
See associated ticket for a full description of the issue.
tl;dr it will be more consistent for our CLI to resolve BQ datasets and tables using the format:
[project_id]:[dataset_id].[opt_table_id]This is the format expected by the
bqcommand-line, and should be Terra's default output.At the same time, for anyone who wants to include a Terra reference within BigQuery SQL, they should use the format we used to output, e.g.
[project_id].[dataset_id].[opt_table_id]This PR switches the default behavior and adds a new
--bq-pathoption to specify the SQL-compatible output format.We should notify our Solutions team before this change goes live, as it will change the default BQ resource resolution.
(Note that this PR is intentionally making a breaking change in a core command. This should be frowned upon as a general matter of course! However, I am high confidence that the existing behavior is incorrect to the point of being actively misleading, and as a result the existing resolve command is not likely relied-upon by our small existing user base. The benefit to adjusting sooner than later outweighs the cost of breaking backwards compatibility.)