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

only compute diff if the schema discovery actually succeeded #22377

Merged
merged 2 commits into from
Feb 6, 2023

Conversation

mfsiega-airbyte
Copy link
Contributor

What

Prevent NPE when we try to diff schemas after unsuccessful schema discovery.

How

Previously, we always tried to compute a diff. Now we only compute a diff if the discovery job was successful.

Testing

Added unit test coverage for the unhappy path here.

Copy link
Contributor

@jdpgrailsdev jdpgrailsdev left a comment

Choose a reason for hiding this comment

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

:shipit:

@mfsiega-airbyte mfsiega-airbyte temporarily deployed to more-secrets February 3, 2023 20:35 — with GitHub Actions Inactive
@mfsiega-airbyte mfsiega-airbyte temporarily deployed to more-secrets February 3, 2023 20:36 — with GitHub Actions Inactive
@github-actions
Copy link
Contributor

github-actions bot commented Feb 3, 2023

Airbyte Code Coverage

There is no coverage information present for the Files changed

Total Project Coverage 24.52%

@mfsiega-airbyte mfsiega-airbyte merged commit f3c94fb into master Feb 6, 2023
@mfsiega-airbyte mfsiega-airbyte deleted the msiega/fix-schema-discovery branch February 6, 2023 15:49
letiescanciano added a commit that referenced this pull request Feb 6, 2023
* master:
  Discover worker starts to use API to write schema result (#21875)
  🪟 🎉  Connector Builder Landing Page (#22122)
  Fix pnpm cache path (#22418)
  Add additional shorter setup guides (#22318)
  Source Amazon Ads: fix reports stream records primary keys (#21677)
  Connector acceptance test: Fix discovered catalog caching for different configs (#22301)
  🪟🐛 Make modal scrollable (#21973)
  only compute diff if the schema discovery actually succeeded (#22377)
  Source Klaviyo: fix schema (#22071)
  🪟 🔧 Switch to `pnpm` for package managing (#22053)
  Source Sentry: turn on default availability strategy (#22303)
  Source freshdesk: deduplicate table names (#22164)
letiescanciano added a commit that referenced this pull request Feb 6, 2023
* master: (86 commits)
  Discover worker starts to use API to write schema result (#21875)
  🪟 🎉  Connector Builder Landing Page (#22122)
  Fix pnpm cache path (#22418)
  Add additional shorter setup guides (#22318)
  Source Amazon Ads: fix reports stream records primary keys (#21677)
  Connector acceptance test: Fix discovered catalog caching for different configs (#22301)
  🪟🐛 Make modal scrollable (#21973)
  only compute diff if the schema discovery actually succeeded (#22377)
  Source Klaviyo: fix schema (#22071)
  🪟 🔧 Switch to `pnpm` for package managing (#22053)
  Source Sentry: turn on default availability strategy (#22303)
  Source freshdesk: deduplicate table names (#22164)
  Update connector-acceptance-tests-reference.md (#22370)
  Update the default security groups for the EC2 runner (#22347)
  Trace refresh schema operations (#22326)
  Remove manual docker upgrades from workflows (#22344)
  Update CODEOWNERS for connector acceptance tests to connector ops (#22341)
  🐛 source: airtable - handle singleSelect types (#22311)
  Source tiktok: chunk advertiser IDs (#22309)
  🪟 🧪 E2E Tests for auto-detect schema changes (#20682)
  ...
letiescanciano added a commit that referenced this pull request Feb 6, 2023
* master: (24 commits)
  Discover worker starts to use API to write schema result (#21875)
  🪟 🎉  Connector Builder Landing Page (#22122)
  Fix pnpm cache path (#22418)
  Add additional shorter setup guides (#22318)
  Source Amazon Ads: fix reports stream records primary keys (#21677)
  Connector acceptance test: Fix discovered catalog caching for different configs (#22301)
  🪟🐛 Make modal scrollable (#21973)
  only compute diff if the schema discovery actually succeeded (#22377)
  Source Klaviyo: fix schema (#22071)
  🪟 🔧 Switch to `pnpm` for package managing (#22053)
  Source Sentry: turn on default availability strategy (#22303)
  Source freshdesk: deduplicate table names (#22164)
  Update connector-acceptance-tests-reference.md (#22370)
  Update the default security groups for the EC2 runner (#22347)
  Trace refresh schema operations (#22326)
  Remove manual docker upgrades from workflows (#22344)
  Update CODEOWNERS for connector acceptance tests to connector ops (#22341)
  🐛 source: airtable - handle singleSelect types (#22311)
  Source tiktok: chunk advertiser IDs (#22309)
  🪟 🧪 E2E Tests for auto-detect schema changes (#20682)
  ...
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.

3 participants