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

replace guava with java8 native #14

Merged
merged 1 commit into from
Jun 24, 2018

Conversation

skuehn
Copy link
Contributor

@skuehn skuehn commented Jun 23, 2018

This change replaces guava with java native alternatives. The intent is to make this library a bit more portable for runtime environments that force old incompatible versions of guava (such as spark and hadoop).

Some PR notes: there isn't a clear alternative to VisibleForTesting, but since the annotation isn't parsed anywhere I figured a comment was functionally equivalent. There isn't a great Iterables.partition built-in in java8 that preserves ordering, so I used a basic iterating subList approach that should be readable at a glance.

FWIW so far I've worked around the guava compatibility issue by relocating the guava packages, and it seems to work fine, but relocation is a bit clunky and removing guava was straight forward.

@azagniotov
Copy link
Owner

@skuehn thanks for the PR!
I think you can still use Java 8 Stream API to partition, e.g.: something across the following lines:

 final int[] itemCounter = new int[]{0};
 final Collection<List<MetricDatum>> metricDataPartitions = metricData.stream()
    .collect(Collectors.groupingBy(item -> itemCounter[0]++ / MAXIMUM_DATUMS_PER_REQUEST))
    .values();

@azagniotov azagniotov merged commit 453fd00 into azagniotov:master Jun 24, 2018
@azagniotov
Copy link
Owner

azagniotov commented Jun 24, 2018

@skuehn I released version 1.0.8 to Maven Central. Please allow a few hours for it to become live

@skuehn
Copy link
Contributor Author

skuehn commented Jun 24, 2018

Awesome - thanks for the quick response and for sharing and maintaining this project. Do you want a PR with the Collectors.groupingBy approach? FWIW I thought the stratified list creation (via dividing by max datums) was a bit odd so I avoided it. Regardless let me know if you want the fix.

@azagniotov
Copy link
Owner

azagniotov commented Jun 25, 2018 via email

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.

2 participants