Skip to content

Commit

Permalink
Require defaultLocation to be set in BigQuery (#1353)
Browse files Browse the repository at this point in the history
* Enforce defaultLocation requirement in core and cli initialization

* Add a test

* Bump version
  • Loading branch information
lewish authored and Ekrekr committed Oct 27, 2023
1 parent 0fb5d53 commit 0149c9e
Show file tree
Hide file tree
Showing 11 changed files with 116 additions and 50 deletions.
24 changes: 24 additions & 0 deletions cli/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,7 @@ const timeoutOption: INamedOption<yargs.Options> = {
};

const defaultDatabaseOptionName = "default-database";
const defaultLocationOptionName = "default-location";
const skipInstallOptionName = "skip-install";
const includeSchedulesOptionName = "include-schedules";
const includeEnvironmentsOptionName = "include-environments";
Expand Down Expand Up @@ -264,6 +265,28 @@ export function runCli() {
}
}
},
{
name: defaultLocationOptionName,
option: {
describe:
"The default BigQuery location to use. See https://cloud.google.com/bigquery/docs/locations for supported values."
},
check: (argv: yargs.Arguments<any>) => {
if (
argv[defaultLocationOptionName] &&
!["bigquery"].includes(argv[warehouseOption.name])
) {
throw new Error(
`The --${defaultLocationOptionName} flag is only used for BigQuery.`
);
}
if (!argv[defaultLocationOptionName] && argv[warehouseOption.name] === "bigquery") {
throw new Error(
`The --${defaultLocationOptionName} flag is required for BigQuery projects. Please run 'dataform help init' for more information.`
);
}
}
},
{
name: skipInstallOptionName,
option: {
Expand Down Expand Up @@ -293,6 +316,7 @@ export function runCli() {
{
warehouse: argv[warehouseOption.name],
defaultDatabase: argv[defaultDatabaseOptionName],
defaultLocation: argv[defaultLocationOptionName],
useRunCache: false
},
{
Expand Down
39 changes: 28 additions & 11 deletions core/session.ts
Original file line number Diff line number Diff line change
Expand Up @@ -375,6 +375,13 @@ export class Session {

public compile(): dataform.CompiledGraph {
this.indexedActions = new ActionIndex(this.adapter(), this.actions);

if (this.config.warehouse === "bigquery" && !this.config.defaultLocation) {
this.compileError(
"A defaultLocation is required for BigQuery. This can be configured in dataform.json."
);
}

const compiledGraph = dataform.CompiledGraph.create({
projectConfig: this.config,
tables: this.compileGraphChunk(
Expand Down Expand Up @@ -615,8 +622,11 @@ export class Session {
}

// materialized
if(!!table.materialized){
if (table.type !== "view" || (this.config.warehouse !== "snowflake" && this.config.warehouse !== "bigquery")) {
if (!!table.materialized) {
if (
table.type !== "view" ||
(this.config.warehouse !== "snowflake" && this.config.warehouse !== "bigquery")
) {
this.compileError(
new Error(`The 'materialized' option is only valid for Snowflake and BigQuery views`),
table.fileName,
Expand Down Expand Up @@ -735,35 +745,42 @@ export class Session {
// BigQuery config
if (!!table.bigquery) {
if (
(table.bigquery.partitionBy || table.bigquery.clusterBy?.length || table.bigquery.partitionExpirationDays
|| table.bigquery.requirePartitionFilter) &&
(table.bigquery.partitionBy ||
table.bigquery.clusterBy?.length ||
table.bigquery.partitionExpirationDays ||
table.bigquery.requirePartitionFilter) &&
table.type === "view"
) {
this.compileError(
`partitionBy/clusterBy/requirePartitionFilter/partitionExpirationDays are not valid for BigQuery views; they are only valid for tables`,
table.fileName,
table.target
);
}
else if (
(!table.bigquery.partitionBy && (table.bigquery.partitionExpirationDays || table.bigquery.requirePartitionFilter)) &&
} else if (
!table.bigquery.partitionBy &&
(table.bigquery.partitionExpirationDays || table.bigquery.requirePartitionFilter) &&
table.type === "table"
) {
this.compileError(
`requirePartitionFilter/partitionExpirationDays are not valid for non partitioned BigQuery tables`,
table.fileName,
table.target
);
}
else if(table.bigquery.additionalOptions) {
if(table.bigquery.partitionExpirationDays && table.bigquery.additionalOptions.partition_expiration_days) {
} else if (table.bigquery.additionalOptions) {
if (
table.bigquery.partitionExpirationDays &&
table.bigquery.additionalOptions.partition_expiration_days
) {
this.compileError(
`partitionExpirationDays has been declared twice`,
table.fileName,
table.target
);
}
if (table.bigquery.requirePartitionFilter && table.bigquery.additionalOptions.require_partition_filter) {
if (
table.bigquery.requirePartitionFilter &&
table.bigquery.additionalOptions.require_partition_filter
) {
this.compileError(
`requirePartitionFilter has been declared twice`,
table.fileName,
Expand Down
3 changes: 2 additions & 1 deletion examples/backwards_compatibility/dataform.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,6 @@
"warehouse": "bigquery",
"defaultDatabase": "tada-analytics",
"defaultSchema": "dataform_example",
"assertionSchema": "dataform_assertions"
"assertionSchema": "dataform_assertions",
"defaultLocation": "US"
}
3 changes: 2 additions & 1 deletion examples/common_v1/dataform.json
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
{
"defaultSchema": "df_integration_test",
"assertionSchema": "df_integration_test_assertions"
"assertionSchema": "df_integration_test_assertions",
"defaultLocation": "US"
}
3 changes: 2 additions & 1 deletion examples/common_v2/dataform.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,6 @@
"defaultDatabase": "tada-analytics",
"defaultSchema": "df_integration_test",
"assertionSchema": "df_integration_test_assertions",
"useRunCache": true
"useRunCache": true,
"defaultLocation": "US"
}
3 changes: 2 additions & 1 deletion examples/never_finishes_compiling/dataform.json
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
{
"warehouse": "redshift",
"defaultSchema": "df_integration_test",
"assertionSchema": "df_integration_test_assertions"
"assertionSchema": "df_integration_test_assertions",
"defaultLocation": "US"
}
44 changes: 23 additions & 21 deletions tests/api/api.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ suite("@dataform/api", () => {
//
// op_d +---> tab_a
const TEST_GRAPH_WITH_TAGS: dataform.ICompiledGraph = dataform.CompiledGraph.create({
projectConfig: { warehouse: "bigquery" },
projectConfig: { warehouse: "bigquery", defaultLocation: "US" },
operations: [
{
target: { schema: "schema", name: "op_a" },
Expand Down Expand Up @@ -260,7 +260,7 @@ suite("@dataform/api", () => {

test("prune removes inline tables", async () => {
const graph: dataform.ICompiledGraph = dataform.CompiledGraph.create({
projectConfig: { warehouse: "bigquery" },
projectConfig: { warehouse: "bigquery", defaultLocation: "US" },
tables: [
{ target: { schema: "schema", name: "a" }, type: "table" },
{
Expand Down Expand Up @@ -379,7 +379,7 @@ suite("@dataform/api", () => {
suite("sql_generating", () => {
test("bigquery_incremental", () => {
const graph = dataform.CompiledGraph.create({
projectConfig: { warehouse: "bigquery", defaultDatabase: "deeb" },
projectConfig: { warehouse: "bigquery", defaultDatabase: "deeb", defaultLocation: "US" },
tables: [
{
target: {
Expand Down Expand Up @@ -428,7 +428,7 @@ suite("@dataform/api", () => {

test("bigquery_materialized", () => {
const testGraph: dataform.ICompiledGraph = dataform.CompiledGraph.create({
projectConfig: { warehouse: "bigquery", defaultDatabase: "deeb" },
projectConfig: { warehouse: "bigquery", defaultDatabase: "deeb", defaultLocation: "US" },
tables: [
{
target: {
Expand Down Expand Up @@ -492,7 +492,7 @@ suite("@dataform/api", () => {

test("snowflake_materialized", () => {
const testGraph: dataform.ICompiledGraph = dataform.CompiledGraph.create({
projectConfig: { warehouse: "snowflake"},
projectConfig: { warehouse: "snowflake" },
tables: [
{
target: {
Expand Down Expand Up @@ -524,8 +524,7 @@ suite("@dataform/api", () => {
tasks: [
{
type: "statement",
statement:
`create or replace materialized view "schema"."materialized" as select 1 as test`
statement: `create or replace materialized view "schema"."materialized" as select 1 as test`
}
],
dependencyTargets: [],
Expand Down Expand Up @@ -716,7 +715,7 @@ suite("@dataform/api", () => {

test("bigquery_partitionby", () => {
const testGraph: dataform.ICompiledGraph = dataform.CompiledGraph.create({
projectConfig: { warehouse: "bigquery", defaultDatabase: "deeb" },
projectConfig: { warehouse: "bigquery", defaultDatabase: "deeb", defaultLocation: "US" },
tables: [
{
target: {
Expand Down Expand Up @@ -783,7 +782,7 @@ suite("@dataform/api", () => {

test("bigquery_options", () => {
const testGraph: dataform.ICompiledGraph = dataform.CompiledGraph.create({
projectConfig: { warehouse: "bigquery", defaultDatabase: "deeb" },
projectConfig: { warehouse: "bigquery", defaultDatabase: "deeb", defaultLocation: "US" },
tables: [
{
target: {
Expand All @@ -795,8 +794,8 @@ suite("@dataform/api", () => {
bigquery: {
partitionBy: "DATE(test)",
clusterBy: [],
partitionExpirationDays : 1,
requirePartitionFilter : true
partitionExpirationDays: 1,
requirePartitionFilter: true
}
},
{
Expand Down Expand Up @@ -852,7 +851,7 @@ suite("@dataform/api", () => {

test("bigquery_clusterby", () => {
const testGraph: dataform.ICompiledGraph = dataform.CompiledGraph.create({
projectConfig: { warehouse: "bigquery", defaultDatabase: "deeb" },
projectConfig: { warehouse: "bigquery", defaultDatabase: "deeb", defaultLocation: "US" },
tables: [
{
target: {
Expand Down Expand Up @@ -919,7 +918,7 @@ suite("@dataform/api", () => {

test("bigquery_additional_options", () => {
const testGraph: dataform.ICompiledGraph = dataform.CompiledGraph.create({
projectConfig: { warehouse: "bigquery", defaultDatabase: "deeb" },
projectConfig: { warehouse: "bigquery", defaultDatabase: "deeb", defaultLocation: "US" },
tables: [
{
target: {
Expand All @@ -929,10 +928,10 @@ suite("@dataform/api", () => {
type: "table",
query: "select 1 as test",
bigquery: {
additionalOptions : {
partition_expiration_days : "1",
"require_partition_filter" : "true",
friendly_name : '"friendlyName"',
additionalOptions: {
partition_expiration_days: "1",
require_partition_filter: "true",
friendly_name: '"friendlyName"'
}
}
},
Expand All @@ -958,7 +957,7 @@ suite("@dataform/api", () => {
{
type: "statement",
statement:
"create or replace table `deeb.schema.additional_options` OPTIONS(partition_expiration_days=1,require_partition_filter=true,friendly_name=\"friendlyName\")as select 1 as test"
'create or replace table `deeb.schema.additional_options` OPTIONS(partition_expiration_days=1,require_partition_filter=true,friendly_name="friendlyName")as select 1 as test'
}
],
dependencyTargets: [],
Expand Down Expand Up @@ -1114,7 +1113,8 @@ suite("@dataform/api", () => {
warehouse: "bigquery",
defaultSchema: "foo",
assertionSchema: "bar",
defaultDatabase: "database"
defaultDatabase: "database",
defaultLocation: "US"
},
runConfig: {
fullRefresh: true
Expand Down Expand Up @@ -1469,7 +1469,8 @@ suite("@dataform/api", () => {
projectConfig: {
warehouse: "bigquery",
defaultSchema: "foo",
assertionSchema: "bar"
assertionSchema: "bar",
defaultLocation: "US"
},
warehouseState: {
tables: []
Expand Down Expand Up @@ -1530,7 +1531,8 @@ suite("@dataform/api", () => {
projectConfig: {
warehouse: "bigquery",
defaultSchema: "foo",
assertionSchema: "bar"
assertionSchema: "bar",
defaultLocation: "US"
},
warehouseState: {
tables: []
Expand Down
6 changes: 5 additions & 1 deletion tests/cli/cli.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,9 @@ suite(__filename, () => {
projectDir,
"--skip-install",
"--default-database",
"dataform-integration-tests"
"dataform-integration-tests",
"--default-location",
"US"
])
);

Expand Down Expand Up @@ -91,6 +93,7 @@ select 1 as \${dataform.projectConfig.vars.testVar2}
defaultSchema: "dataform",
assertionSchema: "dataform_assertions",
defaultDatabase: "dataform-integration-tests",
defaultLocation: "US",
useRunCache: false,
vars: {
testVar1: "testValue1",
Expand Down Expand Up @@ -148,6 +151,7 @@ select 1 as \${dataform.projectConfig.vars.testVar2}
projectConfig: {
assertionSchema: "dataform_assertions",
defaultDatabase: "dataform-integration-tests",
defaultLocation: "US",
defaultSchema: "dataform",
useRunCache: false,
warehouse: "bigquery",
Expand Down
Loading

0 comments on commit 0149c9e

Please sign in to comment.