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.

Sign up for GitHub

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

Jump to bottom

feat(ingestion/dynamodb): implement pagination for list_tables #8910

Merged
merged 12 commits into from
Oct 5, 2023
Merged

feat(ingestion/dynamodb): implement pagination for list_tables #8910

merged 12 commits into from
Oct 5, 2023

Conversation

jinlintt
Copy link
Contributor

@jinlintt jinlintt commented Sep 26, 2023 •
edited
Loading

Also addressed an issue in handling the include_table_item config where the same table names in two different regions could clash. The fix is to specify the table name in the format of region.table_name in the config and also does the matching this way in the code.

Lastly, use the region.table_name when applying the table pattern.

@github-actions github-actions bot added the ingestion PR or Issue related to the ingestion of metadata label Sep 26, 2023
@jinlintt
Copy link
Contributor Author

@asikowitz @mayurinehate could one of you take a look at this?

@jinlintt
Copy link
Contributor Author

@mayurinehate thank you for reviewing. I have addressed your feedback. Please take another look when you get a chance.

mayurinehate
mayurinehate approved these changes Sep 28, 2023
View reviewed changes
Copy link
Collaborator

@mayurinehate mayurinehate left a comment

Choose a reason for hiding this comment

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

LGTM

@jinlintt
Copy link
Contributor Author

Thank you @mayurinehate for reviewing. @asikowitz could you merge it?

@asikowitz asikowitz added the merge-pending-ci A PR that has passed review and should be merged once CI is green. label Sep 28, 2023
@hsheth2 hsheth2 added the community-contribution PR or Issue raised by member(s) of DataHub Community label Sep 29, 2023
@jinlintt
Copy link
Contributor Author

jinlintt commented Oct 4, 2023

@asikowitz @anshbansal can we merge this now that the CI all seem to pass.
The frontend docker image failure doesn't seem to be related.

@anshbansal anshbansal merged commit 6310e51 into datahub-project:master Oct 5, 2023
57 of 58 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@asikowitz asikowitz asikowitz approved these changes

@mayurinehate mayurinehate mayurinehate approved these changes

Assignees
No one assigned
Labels
community-contribution PR or Issue raised by member(s) of DataHub Community ingestion PR or Issue related to the ingestion of metadata merge-pending-ci A PR that has passed review and should be merged once CI is green.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@jinlintt @asikowitz @mayurinehate @anshbansal @hsheth2 @maggiehays