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

Change microbatch lookback default from 0 to 1 #10876

Merged
merged 5 commits into from
Oct 24, 2024

Conversation

QMalcolm
Copy link
Contributor

Resolves #10867

Problem

A lookback value of 0 results in microbatch models consistently having incomplete data.

Solution

Change the lookback default to 1. This guarantees that batches will eventually get completed (assuming the job is run once per batch_size)

Checklist

  • I have read the contributing guide and understand what's expected of me.
  • I have run this code in development, and it appears to resolve the stated issue.
  • This PR includes tests, or tests are not required or relevant for this PR.
  • This PR has no interface changes (e.g., macros, CLI, logs, JSON artifacts, config files, adapter interface, etc.) or this PR has already received feedback and approval from Product or DX.
  • This PR includes type annotations for new and modified functions.

@QMalcolm QMalcolm added the artifact_minor_upgrade To bypass the CI check by confirming that the change is not breaking label Oct 17, 2024
@QMalcolm QMalcolm requested review from a team as code owners October 17, 2024 20:52
@QMalcolm QMalcolm requested review from PaulVPham and removed request for a team October 17, 2024 20:52
@cla-bot cla-bot bot added the cla:yes label Oct 17, 2024
Copy link

codecov bot commented Oct 17, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.12%. Comparing base (bdb79e8) to head (8120810).
Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #10876      +/-   ##
==========================================
- Coverage   89.20%   89.12%   -0.08%     
==========================================
  Files         183      183              
  Lines       23464    23464              
==========================================
- Hits        20930    20913      -17     
- Misses       2534     2551      +17     
Flag Coverage Δ
integration 86.41% <100.00%> (-0.15%) ⬇️
unit 62.06% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Components Coverage Δ
Unit Tests 62.06% <100.00%> (ø)
Integration Tests 86.41% <100.00%> (-0.15%) ⬇️

@QMalcolm
Copy link
Contributor Author

Changes to schemas.getdbt.com dbt-labs/schemas.getdbt.com#68

@mirnawong1
Copy link
Contributor

hey @QMalcolm , when this pr is merged, am i right in changing the docs here from 0 to1? https://docs.getdbt.com/docs/build/incremental-microbatch#relevant-configs

@QMalcolm
Copy link
Contributor Author

hey @QMalcolm , when this pr is merged, am i right in changing the docs here from 0 to1? https://docs.getdbt.com/docs/build/incremental-microbatch#relevant-configs

That is correct 🙂

@mirnawong1
Copy link
Contributor

docs pr here to address this: https://github.com/dbt-labs/docs.getdbt.com/pull/6351/files

@QMalcolm
Copy link
Contributor Author

Something weird happened. Some commits from another one of my branches got onto this one. I assume in my delirium of spinning plates, I did something dumb.

@QMalcolm QMalcolm force-pushed the qmalcolm--10867-default-lookback-to-1 branch from 02d44f4 to bbb8e7e Compare October 24, 2024 17:05
@QMalcolm QMalcolm force-pushed the qmalcolm--10867-default-lookback-to-1 branch from bbb8e7e to 8120810 Compare October 24, 2024 17:08
@QMalcolm QMalcolm merged commit d07bfda into main Oct 24, 2024
58 of 60 checks passed
@QMalcolm QMalcolm deleted the qmalcolm--10867-default-lookback-to-1 branch October 24, 2024 22:16
@mirnawong1
Copy link
Contributor

hey @QMalcolm ! now that this is merged, am i ok merging the docs pr so the docs are updated?

runleonarun pushed a commit to dbt-labs/docs.getdbt.com that referenced this pull request Oct 29, 2024
this pr adds updates to incremental microbatch per core prs:

- [#10878](dbt-labs/dbt-core#10878) - makes it
so --event-time-start adn --event-time-end are mutually required.
- [#10876](dbt-labs/dbt-core#10876) - changes
lookback default window to 1 (from 0)

[ X ] dbt Core PRs must get merged first before docs pr is merged

<!-- vercel-deployment-preview -->
---
🚀 Deployment available! Here are the direct links to the updated files:


-
https://docs-getdbt-com-git-update-microbatch-dbt-labs.vercel.app/docs/build/incremental-microbatch

<!-- end-vercel-deployment-preview -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
artifact_minor_upgrade To bypass the CI check by confirming that the change is not breaking cla:yes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] Defaulting lookback to 0 results in consistently incomplete batches
3 participants