-
Notifications
You must be signed in to change notification settings - Fork 4k
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
bugfix infinite pagination in CDK #3366
Conversation
@@ -21,7 +21,7 @@ class AirbytePythonPlugin implements Plugin<Project> { | |||
installVirtualenv = true | |||
pip 'flake8:3.8.4' | |||
pip 'black:20.8b1' | |||
pip 'mypy:0.790' | |||
pip 'mypy:0.812' |
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.
bump gradle mypy to latest version so running something like gradle format
doesn't overwrite with an older mypy version
def test_stub_next_page_token_http_stream_read_records(mocker): | ||
|
||
stream = StubNextPageTokenHttpStream() | ||
def test_pagination(mocker): |
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.
can we have a more expressive name?
getattr(stream, method).assert_any_call(next_page_token=None, stream_slice=None, stream_state={}) # Assert first call happened | ||
for token in expected_next_page_tokens: | ||
getattr(stream, method).assert_any_call(next_page_token=token, stream_slice=None, stream_state={}) | ||
|
||
assert [{"data": 1}, {"data": 2}, {"data": 3}, {"data": 4}, {"data": 5}, {"data": 6}] == records |
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.
or at least get message here why we expect these data and better move expected to separate variable
/publish-cdk dry-run=false
|
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.
lgtm
What
Closes #3363
How
The CDK incorrectly set
next_page_token
toNone
at the beginning of every pagination loop iteration. Fix this by not settingnext_page_token
inside the loop.