Skip to content

Add TRANSACTION_ISOLATION_LEVEL config in cloudsql-postgres and cloudsql-mysql plugins for develop#675

Open
vedanshugarg04 wants to merge 4 commits into
data-integrations:developfrom
cloudsufi:transaction-isolation-level-develop
Open

Add TRANSACTION_ISOLATION_LEVEL config in cloudsql-postgres and cloudsql-mysql plugins for develop#675
vedanshugarg04 wants to merge 4 commits into
data-integrations:developfrom
cloudsufi:transaction-isolation-level-develop

Conversation

@vedanshugarg04

@vedanshugarg04 vedanshugarg04 commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

Add TRANSACTION_ISOLATION_LEVEL config in cloudsql-postgres and cloudsql-mysql plugins

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request introduces the 'Transaction Isolation Level' configuration option for CloudSQL MySQL and CloudSQL PostgreSQL plugins, updating their batch sinks, batch sources, connectors, and documentation. The review feedback highlights several improvement opportunities, including changing the visibility of the new configuration field in CloudSQLMySQLSource from public to private for better encapsulation, removing redundant lines, correcting typos (such as 'databse'), and ensuring consistent colon usage in the documentation files.

@Name(TRANSACTION_ISOLATION_LEVEL)
@Description("Transaction isolation level for queries run by this source.")
@Nullable
public String transactionIsolationLevel;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The field transactionIsolationLevel is declared as public, whereas other configuration fields in this class (and the corresponding field in CloudSQLPostgreSQLSource) are declared as private. For consistency and better encapsulation, please change its visibility to private.

Suggested change
public String transactionIsolationLevel;
private String transactionIsolationLevel;

Comment on lines +45 to +46
**Transaction Isolation Level:** Transaction isolation level for queries run by this sink.
**Transaction Isolation Level** The transaction isolation level of the database connection

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

These two lines are redundant and contain a missing colon. They can be merged into a single, clean description.

Suggested change
**Transaction Isolation Level:** Transaction isolation level for queries run by this sink.
**Transaction Isolation Level** The transaction isolation level of the database connection
**Transaction Isolation Level:** The transaction isolation level of the database connection.


**Password:** Password to use to connect to the specified database.

**Transaction Isolation Level** The transaction isolation level of the database connection

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

Add a colon after the parameter name for consistency with other documented properties (e.g., **Password:**).

Suggested change
**Transaction Isolation Level** The transaction isolation level of the database connection
**Transaction Isolation Level:** The transaction isolation level of the database connection.


**Password:** Password to use to connect to the specified database.

**Transaction Isolation Level** The transaction isolation level of the database connection

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

Add a colon after the parameter name for consistency with other documented properties (e.g., **Password:**).

Suggested change
**Transaction Isolation Level** The transaction isolation level of the database connection
**Transaction Isolation Level:** The transaction isolation level of the database connection.

Comment on lines +45 to +46
**Transaction Isolation Level:** Transaction isolation level for queries run by this sink.
**Transaction Isolation Level** The transaction isolation level of the databse connection

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

These two lines are redundant and contain a typo ('databse' instead of 'database'). They can be merged into a single, clean description.

Suggested change
**Transaction Isolation Level:** Transaction isolation level for queries run by this sink.
**Transaction Isolation Level** The transaction isolation level of the databse connection
**Transaction Isolation Level:** The transaction isolation level of the database connection.


**Password:** Password to use to connect to the specified database.

**Transaction Isolation Level** The transaction isolation level of the databse connection

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

Fix the typo 'databse' to 'database' and add a colon after the parameter name for consistency.

Suggested change
**Transaction Isolation Level** The transaction isolation level of the databse connection
**Transaction Isolation Level:** The transaction isolation level of the database connection.


**Password:** Password to use to connect to the specified database.

**Transaction Isolation Level** The transaction isolation level of the databse connection

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

Fix the typo 'databse' to 'database' and add a colon after the parameter name for consistency.

Suggested change
**Transaction Isolation Level** The transaction isolation level of the databse connection
**Transaction Isolation Level:** The transaction isolation level of the database connection.

@vedanshugarg04 vedanshugarg04 force-pushed the transaction-isolation-level-develop branch from bfd80f6 to 552c462 Compare June 29, 2026 12:41
@vikasrathee-cs vikasrathee-cs marked this pull request as ready for review June 29, 2026 12:53
Comment thread cloudsql-postgresql-plugin/widgets/CloudSQLPostgreSQL-batchsink.json Outdated
"label": "Transaction Isolation Level",
"name": "transactionIsolationLevel",
"widget-attributes": {
"default": "TRANSACTION_READ_COMMITTED",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This should not change, keep it as it is. and keep the documentation also same

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants