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

Expose onIncompatibleSchemaChange properties in materialization workflows #1372

Merged
merged 36 commits into from
Jan 6, 2025

Conversation

kiahna-tucker
Copy link
Member

@kiahna-tucker kiahna-tucker commented Nov 22, 2024

Issues

The issues directly below are completely resolved by this PR:
#1375

Changes

1375

The following features are included in this PR:

  • Enable the specification-level onIncompatibleSchemaChange property to be configured in materialization workflows. An autocomplete field located in the top-level Backfill section accomplishes this.

  • Create a hook which updates the specification-level onIncompatibleSchemaChange property.

  • Extend BindingState with an onIncompatibleSchemaChange property that captures the value of the specification-level onIncompatibleSchemaChange property.

  • Create a utility function that updates or removes the specification-level onIncompatibleSchemaChange property.

  • Enable the binding-level onIncompatibleSchemaChange property to be configured in materialization workflows. An autocomplete field located in the binding-level Backfill section accomplishes this.

  • Rename the hook which updates the binding-level onIncompatibleSchemaChange property.

  • Extend BindingState with a ResourceConfig['meta']['onIncompatibleSchemaChange'] property that captures the value of the binding-level onIncompatibleSchemaChange property.

Tests

Manually tested

Approaches to testing are as follows:

  • Validate that the incompatible schema change section does not appear in capture workflows.

  • Validate that the field initialized properly when editing a materialization that has the specification-level, onIncompatibleSchemaChange property set to a supported value.

  • Validate that the drafted specification is updated properly when the field is set to a supported value and cleared.

Automated tests

N/A

Playwright tests ran locally

  • Admin
  • Captures
  • Collections
  • HomePage
  • Login
  • Materialization

Screenshots

This field is positioned between the top-level Backfill section and the binding selector.

Spec-level, onIncompatibleSchemaChange field | Empty

pr_screenshot-1372-incompatible_schema-spec_level-empty

Spec-level, onIncompatibleSchemaChange field | Menu options

pr_screenshot-1372-incompatible_schema-spec_level-menu_options

Spec-level, onIncompatibleSchemaChange field | Option selected

pr_screenshot-1372-incompatible_schema-spec_level-option_selected

Spec-level, onIncompatibleSchemaChange field | Unsupported value detected | String

pr_screenshot-1372-incompatible_schema-spec_level-error-invalid_existing_value

Spec-level, onIncompatibleSchemaChange field | Unsupported value detected | Other

pr_screenshot-1372-incompatible_schema-spec_level-error-unsupported_value-object

@kiahna-tucker kiahna-tucker added the change:planned This is a planned change label Nov 22, 2024
@dyaffe
Copy link
Member

dyaffe commented Nov 22, 2024

Thanks, confirming that this looks good to me @kiahna-tucker

@kiahna-tucker kiahna-tucker marked this pull request as ready for review November 25, 2024 14:40
@kiahna-tucker kiahna-tucker requested a review from a team as a code owner November 25, 2024 14:40
@kiahna-tucker kiahna-tucker marked this pull request as draft November 25, 2024 17:54
Copy link
Member

Choose a reason for hiding this comment

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

I struggled with how to name a hook that is for a component... thoughts on keeping the "components are capitalized" rule within the hooks folder?

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm... I think that works for now. It may be difficult to live with as components are often subject to being relocated and renamed, but we can play it by ear.

@kiahna-tucker kiahna-tucker changed the title Introduce a spec-level onIncompatibleSchemaChange field to materialization workflows Expose onIncompatibleSchemaChange properties in materialization workflows Nov 25, 2024
@kiahna-tucker kiahna-tucker marked this pull request as ready for review December 3, 2024 19:18
Copy link
Member

@travjenkins travjenkins left a comment

Choose a reason for hiding this comment

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

overall last round of changes to code look good

still gonna try to break it as best I can with testing

@kiahna-tucker kiahna-tucker merged commit 619acff into main Jan 6, 2025
3 checks passed
@kiahna-tucker kiahna-tucker deleted the kiahna-tucker/incompatible-schema/init-field branch January 6, 2025 16:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
change:planned This is a planned change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants