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

adding in unique_key as a list for incremental models #291

Merged
merged 14 commits into from
Mar 22, 2022

Conversation

McKnight-42
Copy link
Contributor

@McKnight-42 McKnight-42 commented Feb 15, 2022

resolves # CT-134

Description

adding spark implementation of dbt-core recent addition to accept lists as a unique key for incremental models see #4618

Checklist

  • I have signed the CLA
  • I have run this code in development and it appears to resolve the stated issue
  • This PR includes tests, or tests are not required/relevant for this PR
  • I have updated the CHANGELOG.md and added information about my change to the "dbt-spark next" section.

…ake version of possible spark answer to unique keys as a list still researching spark some.
@jtcohen6
Copy link
Contributor

Removing myself from review — feel free to add me back / ping me directly if you have specific questions while working through this!

@McKnight-42 McKnight-42 marked this pull request as ready for review March 11, 2022 21:28
@McKnight-42 McKnight-42 self-assigned this Mar 12, 2022
@McKnight-42
Copy link
Contributor Author

I tried adding in the databricks_cluster versions of the tests to match how we do other integeration tests for spark and they failed checks so I removed them and added changelog still getting some failures from circleci for spec tests

Copy link
Contributor

@VersusFacit VersusFacit left a comment

Choose a reason for hiding this comment

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

I'd be fine with merging as is. I think if you played around with the date fields just a touch more, you could get the other tests working, tbh -- at least that's what I gathered from the circleci docs (the main error was the string to date conversion errors). That way, you could also test against the other endpoint. But I don't think it's that necessary since would the behavior really be that different? Probably not.

@McKnight-42
Copy link
Contributor Author

Date issues found only failing test is unrelated looking into it currently.

@jtcohen6
Copy link
Contributor

@McKnight-42 I think the only remaining failing test is the one I've got a fix for in #301

@McKnight-42
Copy link
Contributor Author

@McKnight-42 I think the only remaining failing test is the one I've got a fix for in #301

great catch, will review and re run tests against

@McKnight-42 McKnight-42 merged commit 5917871 into main Mar 22, 2022
@McKnight-42 McKnight-42 deleted the mcknight/unique_key branch March 22, 2022 14:52
ueshin added a commit to databricks/dbt-databricks that referenced this pull request Mar 25, 2022
### Description

Ports tests for "unique_key as a list" from dbt-labs/dbt-spark#291.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants