Skip to content

Conversation

@bowenli86
Copy link
Member

What is the purpose of the change

currently there are a few ways to get a Properties object with required fields (aws access key, aws secret key, aws region) for KinesisConsumer and KinesisProducer in unit tests. We should unify them and provide a single util to get that Properties object

Brief change log

Unified all similar test utils to TestUtils.getPropertiesWithRequiredFields()

Verifying this change

This change is already covered by existing tests in flink-connector-kinesis

Does this pull request potentially affect one of the following parts:

none

Documentation

none

Copy link
Contributor

@tzulitai tzulitai left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution, changes LGTM once Travis gives green.
One minor nitpick on the name of the refactored method.

/**
* Get properties with Kinesis-required fields.
*/
public static Properties getPropertiesWithRequiredFields() {
Copy link
Contributor

Choose a reason for hiding this comment

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

I prefer the name getStandardProperties over this one. That makes it a bit more coherent with the Kafka test code.

@bowenli86 bowenli86 changed the title [FLINK-8216] Unify test utils in flink-connector-kinesis [FLINK-8216] [Kinesis connector] Unify test utils in flink-connector-kinesis Dec 8, 2017
@tzulitai
Copy link
Contributor

Merging this ..

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants