-
Notifications
You must be signed in to change notification settings - Fork 809
Fixed an issue where connection parameters were using localized values instead of literal values, causing connection failures. #9500 #9517
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
Conversation
WalkthroughRemoved gettext() function wrappers from enum option values in server connection parameter definitions, converting localized enum strings to plain literals while preserving surrounding structure and constraints. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
web/pgadmin/browser/server_groups/servers/static/js/variable.ui.js
Outdated
Show resolved
Hide resolved
…s instead of literal values, causing connection failures. pgadmin-org#9500
eabffde to
3232582
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
web/pgadmin/browser/server_groups/servers/static/js/server.ui.js (1)
56-156: New implementation correctly uses literal enum values for PostgreSQL connection parameters.This new file properly implements connection parameter definitions where enum values are plain string literals (e.g.,
'prefer','require','verify-ca') that PostgreSQL expects, while labels remain localized usinggettext()for UI display.All enum parameters are correctly defined with literal values matching PostgreSQL documentation:
- Authentication/encryption modes:
channel_binding,gssencmode,sslmode,sslnegotiation- Protocol versions:
ssl_min_protocol_version,ssl_max_protocol_version,min_protocol_version,max_protocol_version- Connection behavior:
replication,target_session_attrs,load_balance_hostsThe implementation aligns with backend connection handling in
connection.py, which expects these literal values for PostgreSQL parameter passing.Consider enhancing the regression test suite (
server.ui.spec.js) to include validation that enum values remain as literals in non-English locales to prevent future regressions where enum values might accidentally be wrapped with localization functions.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
web/pgadmin/browser/server_groups/servers/static/js/server.ui.js
🧰 Additional context used
🧬 Code graph analysis (1)
web/pgadmin/browser/server_groups/servers/static/js/server.ui.js (1)
web/regression/javascript/fake_gettext.js (1)
gettext(40-42)
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: run-python-tests-pg (windows-latest, 17)
- GitHub Check: run-python-tests-pg (windows-latest, 14)
- GitHub Check: run-feature-tests-pg (14)
- GitHub Check: run-feature-tests-pg (16)
- GitHub Check: run-feature-tests-pg (18)
- GitHub Check: run-feature-tests-pg (13)
- GitHub Check: run-feature-tests-pg (15)
- GitHub Check: run-feature-tests-pg (17)
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.