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

[FLINK-4032] Replace all usages Guava preconditions #2084

Closed
wants to merge 12 commits into from

Conversation

zentol
Copy link
Contributor

@zentol zentol commented Jun 8, 2016

This PR replaces every usage of the Guava Preconditions in Flink with our own Preconditions class.

In addition,

  • the guava dependency was completely removed from the RabbitMQ connector
  • a checkstyle rules was added preventing further use of guava preconditions

@tzulitai
Copy link
Contributor

tzulitai commented Jun 9, 2016

I think the Kinesis connector is missed out as part of this cleanup. Thanks for this!

@zentol
Copy link
Contributor Author

zentol commented Jun 9, 2016

The Kinesis connector already uses Flink's Preconditions class.

@tzulitai
Copy link
Contributor

tzulitai commented Jun 9, 2016

Ah right, sorry I was mistaken by the guava dependency in pom.xml. The dependency can actually be safely removed also though, must have accidentally left it there when the connector was cleaned up to use Flink's preconditions.

@zentol
Copy link
Contributor Author

zentol commented Jun 9, 2016

It is still used in FlinkKinesisProducer.

@tzulitai
Copy link
Contributor

tzulitai commented Jun 9, 2016

Oops, my mistake again ;) Thanks.

public static void checkElementIndex(int index, int size, @Nullable String errorMessage) {
checkArgument(size >= 0, "Size was negative.");
if (index < 0 || index >= size) {
throw new IndexOutOfBoundsException(errorMessage + " Index: " + index + ", Size: " + size);
Copy link
Contributor

Choose a reason for hiding this comment

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

shouldn't we check if errorMessage == null if it is @Nullable?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

other Preconditions methods print also print "null" if no error message was given. granted, they use String.valueOf(errorMessage), but it's essentially the same.

Copy link
Contributor

Choose a reason for hiding this comment

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

OK

@fhueske
Copy link
Contributor

fhueske commented Jun 15, 2016

Looks good overall. Added just a few minor comments.

@zentol
Copy link
Contributor Author

zentol commented Jun 15, 2016

@fhueske I've addressed your comments.

@fhueske
Copy link
Contributor

fhueske commented Jun 15, 2016

Thanks for the update @zentol. +1 to merge

@zentol
Copy link
Contributor Author

zentol commented Jun 15, 2016

Merging

@asfgit asfgit closed this in c4bbed9 Jun 15, 2016
@zentol zentol deleted the guava branch June 15, 2016 12:06
hequn8128 pushed a commit to hequn8128/flink that referenced this pull request Jun 22, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants