Joffreybvn commented on PR #34669: URL: https://github.com/apache/airflow/pull/34669#issuecomment-1740056888
Thanks for early review ! Mypy is indeed still complaining. I don't have time to fix it right now but maybe tomorrow :) **About your last point**: Yes, i considered the `response_filter`. Initially, I wrote a simple procedure embedded into a PythonOperator; which was using the hook directly. I'd not put that into the `response_filter` function because: - It's simpler to just use a PythonOperator and the hook (instead of bothering with the Operator for the first call) - The response filter is very convenient for filtering responses. I feel hacky when I inject things where its not intended for. "_It would be great to receive a list of Responses in the response_filter_" -> This is the motivation for this PR. **On the name**: It's totally up to you (and other reviewers) how that should be named. Reasoning is the following: We have the SimpleHttpOperator, which is a nice toolset for simple calls. Let's have another toolset for complex calls. It has to start somewhere, so, for now, it has only a pagination feature. With a "_PaginatedOperator_", we don't have a toolset anymore. But it's indeed more clear what the intention is. _Will rename !_ About renaming to `post_call_hook`, I'd say no. For now, the pagination_function is indeed permissive and could be (mis)used. But this function has to define parameters for a next call, and it is used in a loop to repeat calls; thus until there another use-case/PR to modify it, I believe "pagination" is a good name for its behavior. On the other side, I agree with adding another generic `post_call_hook` function right after the call to the hook. It could have been a solution for the SimpleHttpOperator. I can add it ? But so far, my use-case is already covered better by the current state of the PR. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
