-
Notifications
You must be signed in to change notification settings - Fork 34
[🍒] Add TRANSACTION_ISOLATION_LEVEL config in cloudsql-postgres and cloudsql-mysql plugins for release/1.12 #676
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
base: release/1.12
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -152,6 +152,13 @@ public static class CloudSQLMySQLSourceConfig extends AbstractDBSpecificSourceCo | |
| @Description("The existing connection to use.") | ||
| private CloudSQLMySQLConnectorConfig connection; | ||
|
|
||
| @Name(TRANSACTION_ISOLATION_LEVEL) | ||
| @Description("Transaction isolation level for queries run by this source.") | ||
| @Nullable | ||
| private String transactionIsolationLevel; | ||
|
|
||
| private static final String DEFAULT_TRANSACTION_ISOLATION_LEVEL = "TRANSACTION_REPEATABLE_READ"; | ||
|
|
||
| @Override | ||
| protected Map<String, String> getDBSpecificArguments() { | ||
| if (getFetchSize() == null || getFetchSize() <= 0) { | ||
|
|
@@ -164,6 +171,11 @@ protected Map<String, String> getDBSpecificArguments() { | |
| return arguments; | ||
| } | ||
|
|
||
| @Override | ||
| public String getTransactionIsolationLevel() { | ||
| return transactionIsolationLevel != null ? transactionIsolationLevel : DEFAULT_TRANSACTION_ISOLATION_LEVEL; | ||
| } | ||
|
Comment on lines
+174
to
+177
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When Please update the method to delegate to the connection config when @Override
public String getTransactionIsolationLevel() {
if (useConnection != null && useConnection && connection != null) {
return connection.getTransactionIsolationLevel() != null ? connection.getTransactionIsolationLevel() : DEFAULT_TRANSACTION_ISOLATION_LEVEL;
}
return transactionIsolationLevel != null ? transactionIsolationLevel : DEFAULT_TRANSACTION_ISOLATION_LEVEL;
} |
||
|
|
||
| @Override | ||
| public void validate(FailureCollector collector) { | ||
| ConfigUtil.validateConnection(this, useConnection, connection, collector); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -84,6 +84,20 @@ | |
| "label": "Password", | ||
| "name": "password" | ||
| }, | ||
| { | ||
| "widget-type": "select", | ||
| "label": "Transaction Isolation Level", | ||
| "name": "transactionIsolationLevel", | ||
| "widget-attributes": { | ||
| "default": "TRANSACTION_REPEATABLE_READ", | ||
| "values": [ | ||
| "TRANSACTION_REPEATABLE_READ", | ||
| "TRANSACTION_READ_UNCOMMITTED", | ||
| "TRANSACTION_READ_COMMITTED", | ||
| "TRANSACTION_SERIALIZABLE" | ||
| ] | ||
| } | ||
| }, | ||
|
Comment on lines
+87
to
+100
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The Please update the |
||
| { | ||
| "widget-type": "keyvalue", | ||
| "label": "Connection Arguments", | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -153,6 +153,13 @@ public static class CloudSQLPostgreSQLSourceConfig extends AbstractDBSpecificSou | |||||||||||||||||||
| @Description("The existing connection to use.") | ||||||||||||||||||||
| private CloudSQLPostgreSQLConnectorConfig connection; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| @Name(TRANSACTION_ISOLATION_LEVEL) | ||||||||||||||||||||
| @Description("Transaction isolation level for queries run by this source.") | ||||||||||||||||||||
| @Nullable | ||||||||||||||||||||
| private String transactionIsolationLevel; | ||||||||||||||||||||
|
Comment on lines
+156
to
+159
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The
Suggested change
|
||||||||||||||||||||
|
|
||||||||||||||||||||
| private static final String DEFAULT_TRANSACTION_ISOLATION_LEVEL = "TRANSACTION_READ_COMMITTED"; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| @Override | ||||||||||||||||||||
| protected Map<String, String> getDBSpecificArguments() { | ||||||||||||||||||||
| return Collections.emptyMap(); | ||||||||||||||||||||
|
|
@@ -169,6 +176,11 @@ protected CloudSQLPostgreSQLConnectorConfig getConnection() { | |||||||||||||||||||
| return connection; | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| @Override | ||||||||||||||||||||
| public String getTransactionIsolationLevel() { | ||||||||||||||||||||
| return transactionIsolationLevel != null ? transactionIsolationLevel : DEFAULT_TRANSACTION_ISOLATION_LEVEL; | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
Comment on lines
+179
to
+182
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When Please update the method to delegate to the connection config when @Override
public String getTransactionIsolationLevel() {
if (useConnection != null && useConnection && connection != null) {
return connection.getTransactionIsolationLevel() != null ? connection.getTransactionIsolationLevel() : DEFAULT_TRANSACTION_ISOLATION_LEVEL;
}
return transactionIsolationLevel != null ? transactionIsolationLevel : DEFAULT_TRANSACTION_ISOLATION_LEVEL;
} |
||||||||||||||||||||
|
|
||||||||||||||||||||
| @Override | ||||||||||||||||||||
| public void validate(FailureCollector collector) { | ||||||||||||||||||||
| ConfigUtil.validateConnection(this, useConnection, connection, collector); | ||||||||||||||||||||
|
|
||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -84,6 +84,19 @@ | |
| "label": "Password", | ||
| "name": "password" | ||
| }, | ||
| { | ||
| "widget-type": "select", | ||
| "label": "Transaction Isolation Level", | ||
| "name": "transactionIsolationLevel", | ||
| "widget-attributes": { | ||
| "default": "TRANSACTION_READ_COMMITTED", | ||
| "values": [ | ||
| "TRANSACTION_READ_COMMITTED", | ||
| "TRANSACTION_REPEATABLE_READ", | ||
| "TRANSACTION_SERIALIZABLE" | ||
| ] | ||
| } | ||
| }, | ||
|
Comment on lines
+87
to
+99
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The Please update the |
||
| { | ||
| "widget-type": "keyvalue", | ||
| "label": "Connection Arguments", | ||
|
|
||
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.
The
transactionIsolationLevelfield is missing the@Macroannotation. Adding@Macroallows users to dynamically configure the transaction isolation level using pipeline arguments or macros at runtime.