Draft
Conversation
- Implemented connection mode configuration for READ_ONLY, appending "ApplicationIntent=ReadOnly" to the JDBC URL and setting the HikariCP configuration to read-only. - Updated form.json to include a tooltip explaining the implications of using read-only mode. - Added unit tests to verify the correct behavior of both READ_ONLY and READ_WRITE connection modes, ensuring the JDBC URL is constructed appropriately based on the selected mode.
- Simplified the connection mode check by using a local variable for the connection configuration. - Updated unit tests to utilize the new import for READ_ONLY and READ_WRITE modes, ensuring clarity and consistency in the test cases. - Enhanced readability of the code by reducing redundancy in connection mode checks.
Contributor
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
Author
|
/build-deploy-preview skip-tests=true |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/23674174200. |
|
Deploy-Preview-URL: https://ce-41668.dp.appsmith.com |
Failed server tests
|
1 similar comment
Failed server tests
|
Updated the tooltip text for the READ_ONLY connection mode to provide clearer information on its behavior with Always On Availability Groups, emphasizing the potential for write access in certain configurations and the importance of using database roles for strict enforcement.
Contributor
Author
|
/build-deploy-preview skip-tests=true |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/23674513288. |
|
Deploy-Preview-URL: https://ce-41668.dp.appsmith.com |
Failed server tests
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Tip
Add a TL;DR when the description is longer than 500 words or extremely technical (helps the content, marketing, and DevRel team).
Please also include relevant motivation and context. List any dependencies that are required for this change. Add links to Notion, Figma or any other documents that might be relevant to the PR.
This PR wires up the MSSQL datasource’s Read-only connection mode end to end. When the datasource is set to read-only, the plugin adds ApplicationIntent=ReadOnly to the JDBC URL (so SQL Server can send the session to a readable secondary in Always On Availability Groups) and sets Hikari’s pool to read-only. Read/write mode is unchanged and does not add that parameter.
A short tooltip on the connection mode control explains that read-only uses ApplicationIntent=ReadOnly and that it mainly matters for Always On setups. New unit tests check that the built JDBC URL includes ApplicationIntent=ReadOnly for read-only mode and omits ApplicationIntent for read/write.
Fixes #
Issue Numberor
Fixes https://github.com/appsmithorg/appsmith-ee/issues/8615
Warning
If no issue exists, please create an issue first, and check with the maintainers if the issue is valid.
Automation
/ok-to-test tags="@tag.All"
🔍 Cypress test results
Warning
Tests have not run on the HEAD c3d6070 yet
Sat, 28 Mar 2026 01:33:39 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?