Added support for compilation overrides in Data Preparations#1818
Added support for compilation overrides in Data Preparations#1818
Conversation
| // Generate Base64 encoded representation of the YAML. | ||
| const dataPreparationAsObject = loadYaml(resolvedYaml); | ||
| const dataPreparationDefinition = verifyObjectMatchesProto( | ||
| dataform.dataprep.DataPreparation, | ||
| dataPreparationAsObject as { | ||
| [key: string]: any; | ||
| }, | ||
| VerifyProtoErrorBehaviour.DEFAULT | ||
| ); | ||
| const base64encodedContents = encode64( | ||
| dataform.dataprep.DataPreparation, | ||
| dataPreparationDefinition | ||
| ); |
There was a problem hiding this comment.
This seems like quite an indirect way of testing. It would be better to follow the pattern of the other tests, and just write the expected proto output of dataPreparationContents directly, rather than writing a YAML and converting it. The intermediate format isn't important here - just the final entry in the compiled graph.
core/actions/data_preparation.ts
Outdated
| return dataform.dataprep.TableReference.create({ | ||
| project: resolvedTarget.database, | ||
| dataset: resolvedTarget.schema, | ||
| table: resolvedTarget.name | ||
| }) |
There was a problem hiding this comment.
dataset and project may not always be set, so this will end up creating a target proto with potentially empty values.
E.g. I think you need to do:
const dataprepTarget = dataform.dataprep.TableReference.create({table: resolvedTarget.name})
if (resolvedTarget.database) {
dataprepTarget.project = resolvedTarget.database
}
(I'm avoid the spread operator because they can cause uncaught bugs fairly easily).
core/actions/data_preparation.ts
Outdated
| return resolvedDataPreparation; | ||
| } | ||
|
|
||
| function applySessionDefaultsToTableReference( |
There was a problem hiding this comment.
To mirror the naming of applySessionToTarget, this would better be applySessionToTableReference instead
core/actions/data_preparation.ts
Outdated
| } | ||
| } | ||
|
|
||
| function resolveSourcesAndDestinations( |
There was a problem hiding this comment.
Nit: rename to something like applySessionToDataPreparationContents
fernst
deleted the
add-compilation-overrides-support-data-preparation
branch
2 participants
No description provided.