Regex Postgres Connection String #3746
Conversation
regex_connection_string.mp4 |
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
There was a problem hiding this comment.
❌ Changes requested. Reviewed everything up to f77e386 in 1 minute and 36 seconds
More details
- Looked at
18lines of code in1files - Skipped
0files when reviewing. - Skipped posting
0drafted comments based on config settings.
Workflow ID: wflow_RalErTVgvVlH3Crr
Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
⌛ 1 day left in your free trial, upgrade for $20/seat/month or contact us.
| const regex = | ||
| /postgres:\/\/(?<user>[^:@]+)(?::(?<password>[^@]+))?@(?<host>[^:\/?]+)(?::(?<port>\d+))?\/(?<dbname>[^\?]+)?(?:\?.*sslmode=(?<sslmode>[^&]+))?/ | ||
| function parseConnectionString(close: (_: any) => void) { const regex = | ||
| /postgresql?:\/\/(?<user>[^:@]+)(?::(?<password>[^@]+))?@(?<host>[^:\/?]+)(?::(?<port>\d+))?\/(?<dbname>[^\?]+)?(?:\?.*sslmode=(?<sslmode>[^&]+))?/ |
There was a problem hiding this comment.
The regex pattern here is intended to match both 'postgres' and 'postgresql', but it incorrectly matches 'postgresq' as well. Consider using a non-capturing group to correctly match either 'postgres' or 'postgresql'.
| /postgresql?:\/\/(?<user>[^:@]+)(?::(?<password>[^@]+))?@(?<host>[^:\/?]+)(?::(?<port>\d+))?\/(?<dbname>[^\?]+)?(?:\?.*sslmode=(?<sslmode>[^&]+))?/ | |
| /postgres(?:ql)?:\/\/(?<user>[^:@]+)(?::(?<password>[^@]+))?@(?<host>[^:\/?]+)(?::(?<port>\d+))?\/(?<dbname>[^\?]+)?(?:\?.*sslmode=(?<sslmode>[^&]+))?/ |
Deploying windmill with
|
| Latest commit: |
186fa28
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://40e4d081.windmill.pages.dev |
| Branch Preview URL: | https://regex-postgres-cs.windmill.pages.dev |
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on a467c1d in 1 minute and 43 seconds
More details
- Looked at
61lines of code in1files - Skipped
0files when reviewing. - Skipped posting
1drafted comments based on config settings.
1. frontend/src/lib/components/ApiConnectForm.svelte:76
- Draft comment:
The regex pattern correctly supports both 'postgres' and 'postgresql' protocols by using a non-capturing group(?:ql)?to optionally match 'ql'. This allows the regex to match both 'postgres://' and 'postgresql://' in the connection strings. - Reason this comment was not posted:
Confidence changes required:0%
The regex pattern in the parseConnectionString function has been updated to support both 'postgres' and 'postgresql' protocols. However, the regex pattern uses a non-capturing group (?:ql)? which is correct for optional matching of 'ql' in 'postgresql'. The regex pattern is designed to capture different parts of the connection string such as user, password, host, port, dbname, and sslmode. The regex seems to be correctly formed to handle both 'postgres' and 'postgresql' protocols.
Workflow ID: wflow_1b1HFh9AKQueiO66
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
⌛ 1 day left in your free trial, upgrade for $20/seat/month or contact us.
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on 3b66941 in 1 minute and 52 seconds
More details
- Looked at
61lines of code in1files - Skipped
0files when reviewing. - Skipped posting
1drafted comments based on config settings.
1. frontend/src/lib/components/ApiConnectForm.svelte:76
- Draft comment:
The regex pattern here is intended to support both 'postgres' and 'postgresql' protocols according to the PR description, but it currently only supports 'postgres'. Please update the regex to reflect this.
/postgres(?:ql)?://(?<user>[^:@]+)(?::(?<password>[^@]+))?@(?<host>[^:/?]+)(?::(?<port>\d+))?\/(?<dbname>[^\?]+)?(?:\?.*sslmode=(?<sslmode>[^&]+))?/
- Reason this comment was not posted:
Confidence of 0% on close inspection, compared to threshold of 50%.
Workflow ID: wflow_DSWoXHGgjMkpSLhF
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
⌛ 1 day left in your free trial, upgrade for $20/seat/month or contact us.
There was a problem hiding this comment.
❌ Changes requested. Incremental review on 186fa28 in 1 minute and 51 seconds
More details
- Looked at
60lines of code in1files - Skipped
0files when reviewing. - Skipped posting
0drafted comments based on config settings.
Workflow ID: wflow_q7qy6Bv3yKNRdJrn
Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
| const regex = | ||
| /postgres:\/\/(?<user>[^:@]+)(?::(?<password>[^@]+))?@(?<host>[^:\/?]+)(?::(?<port>\d+))?\/(?<dbname>[^\?]+)?(?:\?.*sslmode=(?<sslmode>[^&]+))?/ | ||
| /postgres(?:ql)?:\/\/(?<user>[^:@]+)(?::(?<password>[^@]+))?@(?<host>[^:\/?]+)(?::(?<port>\d+))?\/(?<dbname>[^\?]+)?(?:\?.*sslmode=(?<sslmode>[^&]+))?/ |
There was a problem hiding this comment.
The regex pattern has not been updated to support both 'postgres' and 'postgresql' as described in the PR description. Please update the regex to correctly match both protocols.
| /postgres(?:ql)?:\/\/(?<user>[^:@]+)(?::(?<password>[^@]+))?@(?<host>[^:\/?]+)(?::(?<port>\d+))?\/(?<dbname>[^\?]+)?(?:\?.*sslmode=(?<sslmode>[^&]+))?/ | |
| /postgres(?:ql)?://(?<user>[^:@]+)(?::(?<password>[^@]+))?@(?<host>[^:/?]+)(?::(?<port>\d+))?\/(?<dbname>[^\?]+)?(?:\?.*sslmode=(?<sslmode>[^&]+))?/ |
Summary:
Updated the regex pattern in
ApiConnectForm.svelteto support both 'postgres' and 'postgresql' protocols in connection strings.Key points:
parseConnectionStringfunction inApiConnectForm.svelteto support 'postgresql' in addition to 'postgres'.Generated with ❤️ by ellipsis.dev