Skip to content

Commit

Permalink
Fix publish type backwards compatibility (#1910)
Browse files Browse the repository at this point in the history 
* Progress fixing type backwards compatibility

* Progress to class extension error

* Move reset table site def back to callsites

* Fix class inheritence error

* Fix various type conflict issues

* Fix type parameter conversion

* Fix lint

* Bump version
  • Loading branch information
@Ekrekr
Ekrekr authored Feb 5, 2025
1 parent 167f36a commit 5995f4e
Show file tree
Hide file tree
Showing 9 changed files with 467 additions and 242 deletions.
115 changes: 80 additions & 35 deletions core/actions/incremental_table.ts
View file
Original file line number Diff line number Diff line change
@@ -1,11 +1,15 @@
import { verifyObjectMatchesProto, VerifyProtoErrorBehaviour } from "df/common/protos";
import {
ActionBuilder,
ILegacyTableBigqueryConfig,
ILegacyBigQueryOptions,
ILegacyTableConfig,
ITableContext,
LegacyConfigConverter
LegacyConfigConverter,
TableType
} from "df/core/actions";
import { Assertion } from "df/core/actions/assertion";
import { Table } from "df/core/actions/table";
import { View } from "df/core/actions/view";
import { ColumnDescriptors } from "df/core/column_descriptors";
import { Contextable, Resolvable } from "df/core/common";
import * as Path from "df/core/path";
Expand All @@ -25,25 +29,6 @@ import {
} from "df/core/utils";
import { dataform } from "df/protos/ts";

/**
* @hidden
* This maintains backwards compatability with older versions.
* TODO(ekrekr): consider breaking backwards compatability of these in v4.
*/
export interface ILegacyIncrementalTableConfig
extends dataform.ActionConfig.IncrementalTableConfig {
dependencies: Resolvable[];
database: string;
schema: string;
fileName: string;
type: string;
bigquery?: ILegacyTableBigqueryConfig;
// Legacy incremental table config's table assertions cannot directly extend the protobuf
// incremental table config definition because of legacy incremental table config's flexible
// types.
assertions: any;
}

/**
* @hidden
*/
Expand Down Expand Up @@ -71,9 +56,15 @@ export class IncrementalTable extends ActionBuilder<dataform.Table> {
private uniqueKeyAssertions: Assertion[] = [];
private rowConditionsAssertion: Assertion;

private unverifiedConfig: any;
private configPath: string | undefined;

constructor(session?: Session, unverifiedConfig?: any, configPath?: string) {
super(session);
this.session = session;
this.configPath = configPath;
// A copy is used here to prevent manipulation of the original.
this.unverifiedConfig = Object.assign({}, unverifiedConfig);

if (!unverifiedConfig) {
return;
Expand Down Expand Up @@ -158,11 +149,48 @@ export class IncrementalTable extends ActionBuilder<dataform.Table> {
if (config.filename) {
this.proto.fileName = config.filename;
}
this.proto.onSchemaChange = this.mapOnSchemaChange(config.onSchemaChange)
this.proto.onSchemaChange = this.mapOnSchemaChange(config.onSchemaChange);

return this;
}

/**
* @hidden
* @deprecated
* Deprecated in favor of action type can being set in the configs passed to action constructor
* functions.
*/
public type(type: TableType) {
let newAction: View | Table;
switch (type) {
case "table":
newAction = new Table(
this.session,
{ ...this.unverifiedConfig, type: "table" },
this.configPath
);
break;
case "incremental":
return this;
case "view":
newAction = new View(
this.session,
{ ...this.unverifiedConfig, type: "view" },
this.configPath
);
break;
default:
throw new Error(`Unexpected table type: ${type}`);
}
const existingAction = this.session.actions.indexOf(this);
if (existingAction === -1) {
throw Error(
"Expected pre-existing action, but none found. Please report this to the Dataform team."
);
}
this.session.actions[existingAction] = newAction;
}

public query(query: Contextable<ITableContext, string>) {
this.contextableQuery = query;
return this;
Expand Down Expand Up @@ -398,8 +426,15 @@ export class IncrementalTable extends ActionBuilder<dataform.Table> {
return protoOps;
}

/**
* Verify config checks that the constructor provided config matches the expected proto structure,
* or the previously accepted legacy structure. If the legacy structure is used, it is converted
* to the new structure.
*/
private verifyConfig(
unverifiedConfig: ILegacyIncrementalTableConfig
// `any` is used here to facilitate the type merging of the legacy table config, which is very
// different to the new structure.
unverifiedConfig: dataform.ActionConfig.IncrementalTableConfig | ILegacyTableConfig | any
): dataform.ActionConfig.IncrementalTableConfig {
// The "type" field only exists on legacy incremental table configs. Here we convert them to the
// new format.
Expand Down Expand Up @@ -441,7 +476,7 @@ export class IncrementalTable extends ActionBuilder<dataform.Table> {
throw e;
},
unverifiedConfig.bigquery,
strictKeysOf<ILegacyTableBigqueryConfig>()([
strictKeysOf<ILegacyBigQueryOptions>()([
"partitionBy",
"clusterBy",
"updatePartitionFilter",
Expand Down Expand Up @@ -471,27 +506,37 @@ export class IncrementalTable extends ActionBuilder<dataform.Table> {
// - for sqlx it will have type "string"
// - for action.yaml it will be converted to enum which is represented
// in TypeScript as a "number".
private mapOnSchemaChange(onSchemaChange?: string|number) : dataform.OnSchemaChange {
private mapOnSchemaChange(onSchemaChange?: string | number): dataform.OnSchemaChange {
if (!onSchemaChange) {
return dataform.OnSchemaChange.IGNORE;
}

if (typeof onSchemaChange === "number") {
switch (onSchemaChange) {
case dataform.ActionConfig.OnSchemaChange.IGNORE: return dataform.OnSchemaChange.IGNORE;
case dataform.ActionConfig.OnSchemaChange.FAIL: return dataform.OnSchemaChange.FAIL;
case dataform.ActionConfig.OnSchemaChange.EXTEND: return dataform.OnSchemaChange.EXTEND;
case dataform.ActionConfig.OnSchemaChange.SYNCHRONIZE: return dataform.OnSchemaChange.SYNCHRONIZE;
default: throw new Error(`OnSchemaChange value "${onSchemaChange}" is not supported`);
case dataform.ActionConfig.OnSchemaChange.IGNORE:
return dataform.OnSchemaChange.IGNORE;
case dataform.ActionConfig.OnSchemaChange.FAIL:
return dataform.OnSchemaChange.FAIL;
case dataform.ActionConfig.OnSchemaChange.EXTEND:
return dataform.OnSchemaChange.EXTEND;
case dataform.ActionConfig.OnSchemaChange.SYNCHRONIZE:
return dataform.OnSchemaChange.SYNCHRONIZE;
default:
throw new Error(`OnSchemaChange value "${onSchemaChange}" is not supported`);
}
}

switch (onSchemaChange.toString().toUpperCase()) {
case "IGNORE": return dataform.OnSchemaChange.IGNORE;
case "FAIL": return dataform.OnSchemaChange.FAIL;
case "EXTEND": return dataform.OnSchemaChange.EXTEND;
case "SYNCHRONIZE": return dataform.OnSchemaChange.SYNCHRONIZE;
default: throw new Error(`OnSchemaChange value "${onSchemaChange}" is not supported`);
case "IGNORE":
return dataform.OnSchemaChange.IGNORE;
case "FAIL":
return dataform.OnSchemaChange.FAIL;
case "EXTEND":
return dataform.OnSchemaChange.EXTEND;
case "SYNCHRONIZE":
return dataform.OnSchemaChange.SYNCHRONIZE;
default:
throw new Error(`OnSchemaChange value "${onSchemaChange}" is not supported`);
}
}
}
Expand Down
109 changes: 87 additions & 22 deletions core/actions/index.ts
View file
Original file line number Diff line number Diff line change
@@ -1,12 +1,19 @@
import { Assertion } from "df/core/actions/assertion";
import { DataPreparation } from "df/core/actions/data_preparation";
import { Declaration } from "df/core/actions/declaration";
import { ILegacyIncrementalTableConfig, IncrementalTable } from "df/core/actions/incremental_table";
import { IncrementalTable } from "df/core/actions/incremental_table";
import { Notebook } from "df/core/actions/notebook";
import { Operation } from "df/core/actions/operation";
import { ILegacyTableConfig, Table } from "df/core/actions/table";
import { ILegacyViewConfig, View } from "df/core/actions/view";
import { ICommonContext } from "df/core/common";
import { Table } from "df/core/actions/table";
import { View } from "df/core/actions/view";
import {
IActionConfig,
ICommonContext,
IDependenciesConfig,
IDocumentableConfig,
INamedConfig,
ITargetableConfig
} from "df/core/common";
import { Session } from "df/core/session";
import { dataform } from "df/protos/ts";

Expand Down Expand Up @@ -114,6 +121,71 @@ export interface ITableContext extends ICommonContext {
incremental: () => boolean;
}

/**
* @hidden
* @deprecated
* This is no longer needed other than for legacy backwards compatibility purposes, as tables are
* now configured in separate actions.
*/
export type TableType = typeof TableType[number];

/**
* @hidden
* @deprecated
* This is no longer needed other than for legacy backwards compatibility purposes, as tables are
* now configured in separate actions.
*/
export const TableType = ["table", "view", "incremental"] as const;

/**
* @hidden
* @deprecated
* These options are only here to preserve backwards compatibility of legacy config options.
* consider breaking backwards compatability of this in v4.
*/
export interface ILegacyTableConfig
extends IActionConfig,
IDependenciesConfig,
IDocumentableConfig,
INamedConfig,
ITargetableConfig {
type?: TableType;
protected?: boolean;
bigquery?: ILegacyBigQueryOptions;
assertions?: ILegacyTableAssertions;
uniqueKey?: string[];
materialized?: boolean;
}

/**
* @hidden
* @deprecated
* These options are only here to preserve backwards compatibility of legacy config options.
* consider breaking backwards compatability of this in v4.
*/
export interface ILegacyBigQueryOptions {
partitionBy?: string;
clusterBy?: string[];
updatePartitionFilter?: string;
labels?: { [name: string]: string };
partitionExpirationDays?: number;
requirePartitionFilter?: boolean;
additionalOptions?: { [name: string]: string };
}

/**
* @hidden
* @deprecated
* These options are only here to preserve backwards compatibility of legacy config options.
* consider breaking backwards compatability of this in v4.
*/
export interface ILegacyTableAssertions {
uniqueKey?: string | string[];
uniqueKeys?: string[][];
nonNull?: string | string[];
rowConditions?: string[];
}

export class LegacyConfigConverter {
// This is a workaround to make bigquery options output empty fields with the same behaviour as
// they did previously.
Expand All @@ -137,9 +209,11 @@ export class LegacyConfigConverter {
return bigqueryFiltered;
}

public static insertLegacyInlineAssertionsToConfigProto<
T extends ILegacyTableConfig | ILegacyIncrementalTableConfig | ILegacyViewConfig
>(legacyConfig: T): T {
public static insertLegacyInlineAssertionsToConfigProto<T extends ILegacyTableConfig>(
unverifiedConfig: T
): T {
// Type `any` is used here to facilitate the type hacking for legacy compatibility.
const legacyConfig: any = unverifiedConfig;
if (legacyConfig?.assertions) {
if (typeof legacyConfig.assertions?.uniqueKey === "string") {
legacyConfig.assertions.uniqueKey = [legacyConfig.assertions.uniqueKey];
Expand All @@ -158,9 +232,11 @@ export class LegacyConfigConverter {
return legacyConfig;
}

public static insertLegacyBigQueryOptionsToConfigProto<
T extends ILegacyTableConfig | ILegacyIncrementalTableConfig
>(legacyConfig: T): T {
public static insertLegacyBigQueryOptionsToConfigProto<T extends ILegacyTableConfig>(
unverifiedConfig: T
): T {
// Type `any` is used here to facilitate the type hacking for legacy compatibility.
const legacyConfig: any = unverifiedConfig;
if (!legacyConfig?.bigquery) {
return legacyConfig;
}
Expand All @@ -173,8 +249,7 @@ export class LegacyConfigConverter {
delete legacyConfig.bigquery.clusterBy;
}
if (!!legacyConfig.bigquery.updatePartitionFilter) {
(legacyConfig as ILegacyIncrementalTableConfig).updatePartitionFilter =
legacyConfig.bigquery.updatePartitionFilter;
legacyConfig.updatePartitionFilter = legacyConfig.bigquery.updatePartitionFilter;
delete legacyConfig.bigquery.updatePartitionFilter;
}
if (!!legacyConfig.bigquery.labels) {
Expand All @@ -201,13 +276,3 @@ export class LegacyConfigConverter {
return legacyConfig;
}
}

export interface ILegacyTableBigqueryConfig {
partitionBy?: string;
clusterBy?: string[];
updatePartitionFilter?: string;
labels?: { [key: string]: string };
partitionExpirationDays?: number;
requirePartitionFilter?: boolean;
additionalOptions?: { [key: string]: string };
}
Loading

0 comments on commit 5995f4e

Please sign in to comment.