Skip to content
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

Require defaultLocation to be set in BigQuery #1353

Merged
merged 3 commits into from
Jul 21, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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