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

fix: use dedup on new records for deltalake #3927

Merged
merged 1 commit into from
Sep 29, 2023

Conversation

achettyiitr
Copy link
Member

@achettyiitr achettyiitr commented Sep 28, 2023

Description

  • Use dedup on new records for deltalake

Linear Ticket

Security

  • The code changed/added as part of this pull request won't create any security issues with how the software is being used.

@codecov
Copy link

codecov bot commented Sep 28, 2023

Codecov Report

Attention: 3 lines in your changes are missing coverage. Please review.

Comparison is base (7c626cc) 68.88% compared to head (b47d7b0) 68.88%.

Additional details and impacted files
@@               Coverage Diff               @@
##           release/1.14.x    #3927   +/-   ##
===============================================
  Coverage           68.88%   68.88%           
===============================================
  Files                 351      351           
  Lines               52697    52700    +3     
===============================================
+ Hits                36300    36305    +5     
+ Misses              14084    14081    -3     
- Partials             2313     2314    +1     
Files Coverage Δ
warehouse/integrations/deltalake/deltalake.go 74.27% <0.00%> (-0.23%) ⬇️

... and 4 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@achettyiitr achettyiitr force-pushed the fix.deltalake-etl-dedup branch from 29d0231 to b47d7b0 Compare September 29, 2023 05:03
Comment on lines +902 to +904
if d.Uploader.ShouldOnDedupUseNewRecord() {
return "", nil
}
Copy link
Member Author

Choose a reason for hiding this comment

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

Since the changes are straightforward, going forward with the changes. Also, I have captured the test here since it was easy for me to add it there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants