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

fix: merge api results after their transformations (TCTC-9634) #1807

Merged
merged 4 commits into from
Nov 6, 2024

Conversation

julien-pinchelimouroux
Copy link
Contributor

Why

API results where appended before any user's configured transformations (filters, flatten). We have to append resulted dataframes.
Moreover, APIs can return directly json objects, which created invalid results merging.
In addition, we have to add a data_length_filter to offset_limit pagination in case of the data result isn't relevant to compute the data length.

@julien-pinchelimouroux julien-pinchelimouroux added bug Something isn't working Need Review labels Oct 31, 2024
Copy link
Member

@davinov davinov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, thanks for this fix and the associated test!

Copy link
Contributor

@lukapeschke lukapeschke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! few nits here and there. And pandas.concat should be compatible with v1 and v2, AND do what you want 🙂

toucan_connectors/utils/dataframe.py Outdated Show resolved Hide resolved
toucan_connectors/http_api/http_api_connector.py Outdated Show resolved Hide resolved
toucan_connectors/http_api/pagination_configs.py Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
Copy link
Contributor

@lukapeschke lukapeschke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thx!

@julien-pinchelimouroux julien-pinchelimouroux merged commit 76dff20 into master Nov 6, 2024
4 checks passed
@julien-pinchelimouroux julien-pinchelimouroux deleted the TCTC-9634/fix_pagination_flatten branch November 6, 2024 08:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Need Review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants