-
Notifications
You must be signed in to change notification settings - Fork 4.9k
Fix: source-notion depth leak #69780
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
Conversation
👋 Welcome to Airbyte!Thank you for your contribution from eytan-avisror/airbyte! We're excited to have you in the Airbyte community. Helpful Resources
PR Slash CommandsAs needed or by request, Airbyte Maintainers can execute the following slash commands on your PR:
If you have any questions, feel free to ask in the PR comments or join our Slack community. Tips for Working with CI
|
|
@eytan-avisror Thanks for your contribution! Some of the tests were unable to run since the branch doesn't exist in the airbytehq repo. I've copied over the PR here so we can run all the tests. Overall, the PR looks good! I just changed the unit test name and changelog date for this upcoming Monday, which is when we plan to ship this. I'll leave this PR open till we get the other one merged and follow up if anything comes up! :) |
Sounds good, Thanks @agarctfi |
|
/run-connector-tests
|
agarctfi
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We tested this locally & on cloud and everything looks good!
@DanyloGL was able to get the tests running on this PR so we're gonna use it for the merge.
What
Fix depth counter bug in
BlocksRetrieverthat causedcurrent_block_depthto accumulate incorrectly across sibling blocks with children.The
BlocksRetriever.read_records()method incrementscurrent_block_depthwhen processing blocks with children, but was never decrementing it after the recursive call completes. This causes the depth counter to keep growing instead of returning to the parent level.Impact of the bug:
MAX_BLOCK_DEPTHlimit (30) and stop retrieving valid nested blocksHow
Added
self.current_block_depth -= 1after the recursiveyield from self.read_records()call incomponents.pyline 88 to properly restore the depth counter when returning from child block processing.Before (Buggy behavior):
Review guide
User Impact
Unable to properly ingest from notion, many blocks missing.
Can this PR be safely reverted and rolled back?