Skip to content

Update Data Preparation processor to remove proto dependency. #1839

Merged
fernst merged 2 commits intomainfrom
remove-dataprep-proto-dependency
Sep 11, 2024
Merged

Update Data Preparation processor to remove proto dependency. #1839
fernst merged 2 commits intomainfrom
remove-dataprep-proto-dependency

Conversation

@fernst
Copy link
Collaborator

@fernst fernst commented Sep 11, 2024

The data preparation in the compiled graph now contains the resolved YAML after compiling.

I will remove the proto reference in a follow up PR.

…a preparation in the compiled graph now contains the resolved YAML after compiling.
@fernst fernst requested a review from Ekrekr September 11, 2024 14:52
definition.nodes.forEach((node, index) => {
// Loop through all nodes and resolve the compilation overrides for
// all source and destination tables.
if (definition.nodes) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that definition.nodes isn't guaranteed to be present, so this may throw a weird error to the user if they fill out the YAML wrong

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should I try catching any errors here and then rethrowing with an error message mentioning that the YAML is invalid?

@Ekrekr
Copy link
Contributor

Ekrekr commented Sep 11, 2024

(Fix tests before merge plz)

@fernst fernst merged commit 86254eb into main Sep 11, 2024
@fernst fernst deleted the remove-dataprep-proto-dependency branch September 11, 2024 19:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants