-
Notifications
You must be signed in to change notification settings - Fork 57
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
chore: update Checkstyle version and fix errors #304
Conversation
@Test | ||
public void testValidateTablesGiven() { | ||
final Map<String, String> properties = new HashMap<>(); | ||
properties.put(JdbcSourceTaskConfig.TABLES_CONFIG, "test-table"); | ||
properties.put(JdbcConfig.CONNECTION_URL_CONFIG, "connection-url"); | ||
properties.put(JdbcSourceConnectorConfig.MODE_CONFIG, "bulk"); | ||
properties.put(JdbcSourceConnectorConfig.TOPIC_PREFIX_CONFIG, "test-prefix"); | ||
final JdbcSourceTaskConfig config = new JdbcSourceTaskConfig(properties); | ||
config.validate(); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since its a list I would add in the test also a test regarding the retrieval of the values, something like:
@Test | |
public void testValidateTablesGiven() { | |
final Map<String, String> properties = new HashMap<>(); | |
properties.put(JdbcSourceTaskConfig.TABLES_CONFIG, "test-table"); | |
properties.put(JdbcConfig.CONNECTION_URL_CONFIG, "connection-url"); | |
properties.put(JdbcSourceConnectorConfig.MODE_CONFIG, "bulk"); | |
properties.put(JdbcSourceConnectorConfig.TOPIC_PREFIX_CONFIG, "test-prefix"); | |
final JdbcSourceTaskConfig config = new JdbcSourceTaskConfig(properties); | |
config.validate(); | |
} | |
@Test | |
public void testValidateTablesGiven() { | |
final Map<String, String> properties = new HashMap<>(); | |
properties.put(JdbcSourceTaskConfig.TABLES_CONFIG, "test-table1, test-table2"); | |
properties.put(JdbcConfig.CONNECTION_URL_CONFIG, "connection-url"); | |
properties.put(JdbcSourceConnectorConfig.MODE_CONFIG, "bulk"); | |
properties.put(JdbcSourceConnectorConfig.TOPIC_PREFIX_CONFIG, "test-prefix"); | |
final JdbcSourceTaskConfig config = new JdbcSourceTaskConfig(properties); | |
config.validate(); | |
assertEquals(config.getList(JdbcSourceTaskConfig.TABLES_CONFIG), List.of("test-table1", "test-table2")); | |
} |
@@ -31,11 +33,23 @@ public class JdbcSourceTaskConfig extends JdbcSourceConnectorConfig { | |||
|
|||
public static final String TABLES_CONFIG = "tables"; | |||
private static final String TABLES_DOC = "List of tables for this task to watch for changes."; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would like to add: encoded as a comma separated string
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To me looks fine, if you wanna add the suggestion I'm happier, not mandatory. I can merge as it is if you prefer
b0cdc26
to
8dfd1ff
Compare
8dfd1ff
to
60b75b2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
No description provided.