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

Publish the list of partition ids on TABLE_PARTITION_UPDATE event #449

Merged
merged 2 commits into from
Jul 29, 2021

Conversation

insyncoss
Copy link
Collaborator

No description provided.

* @return list of partition ids from the input list
*/
protected static List<String> getPartitionNameListFromDtos(final List<PartitionDto> partitionDtos) {
if (partitionDtos.size() > PARTS_UPDATED_LIST_MAX_SIZE) {
Copy link
Contributor

Choose a reason for hiding this comment

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

We should make PARTS_UPDATED_LIST_MAX_SIZE configurable.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Makes sense

return Collections.emptyList();
}
return partitionDtos.stream()
.map(PartitionDto::getName)
Copy link
Contributor

Choose a reason for hiding this comment

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

Minor: We can as well make one mapper call with dto -> dto.getName().getPartitionName()

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ok

raveeram
raveeram previously approved these changes Jul 25, 2021
@@ -174,6 +179,25 @@ public TablePartitionsUpdatePayload createTablePartitionsUpdatePayload(
.collect(Collectors.toList());
}

/**
* get partition name list from list of partitionDtos. The returned list is capped at
* the first PARTS_UPDATED_LIST_MAX_SIZE elements, if there are more than that number of elements
Copy link
Contributor

Choose a reason for hiding this comment

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

should we just call this PARTITIONS_UPDATED_LIST_MAX_SIZE

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ok changed

Copy link
Contributor

@ArtfulCoder ArtfulCoder left a comment

Choose a reason for hiding this comment

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

Thank you!
The max partition size will be made configurable in a future PR.

@insyncoss insyncoss merged commit 7c64d4a into master Jul 29, 2021
@insyncoss insyncoss deleted the publish-parts branch September 15, 2021 22:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants