-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
[low code connectors] fix bug where headers were not passed to cursor interpolation #15347
[low code connectors] fix bug where headers were not passed to cursor interpolation #15347
Conversation
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.
looks good! Where do you think we should document this behavior?
Can you clarify how adding the pre-parsed headers field changes the interface for the developer? What’s the before and after? |
Before:
This is probably impossible for developers to parse without utility functions added in jinja for string manpulation and regexes. Basically they would need to replicate all the behavior of After: YAML config: |
@girarda |
|
@sherifnada that's a good point. Although I don't think the other pagination strategies need or use the response for anything. One area that I do think could get some value from receiving headers is in our error handlers. We do this for some error handling strategies, but I do think there is one scenario not covered in |
@@ -56,7 +56,7 @@ def matches(self, response: requests.Response) -> Optional[ResponseAction]: | |||
return None | |||
|
|||
def _response_matches_predicate(self, response: requests.Response) -> bool: | |||
return self.predicate and self.predicate.eval(None, response=response.json()) | |||
return self.predicate and self.predicate.eval(None, response=response.json(), headers=response.headers) |
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.
should we update the "link" header here to be consistent with the cursor pagination strategy?
What
We were not correctly passing
headers
to the cursor value so when we tried to perform interpolation to get values from headers it would fail.How
Pass the headers to interpolation like we do for the stop condition. I also added a little bit of code to make it easier to extract next urls in an indexable way. The existing implementation on
requests.Response
returns a giant string of links which would be a pain for developers to extract. Relying on the existingResponse.links()
helper method and storing it back onto the response is much more user friendly. +testsRecommended reading order
cursor_pagination_strategy.py