-
Notifications
You must be signed in to change notification settings - Fork 805
Fixed the SSL certificate issue while checking for the upgrade. #9293 #9471
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: master
Are you sure you want to change the base?
Conversation
WalkthroughAdded certifi as a dependency and updated the upgrade_check function to use certifi for SSL certificate verification. The function now creates SSL context with the CA file when available, or falls back to certifi.where() for certificate bundle location when the CA file is missing. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ 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 |
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/misc/__init__.py (1)
385-390: Use consistent parameter passing for maintainability.Line 390 uses positional arguments for
dataandtimeout, while line 387 uses keyword arguments. For consistency and readability, consider using keyword arguments throughout.Apply this diff to improve consistency:
- response = urlopen(url, data, 5, cafile=certifi.where()) + response = urlopen(url, data=data, timeout=5, cafile=certifi.where())
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
requirements.txt(1 hunks)web/pgadmin/misc/__init__.py(2 hunks)
🧰 Additional context used
🪛 Ruff (0.14.8)
web/pgadmin/misc/__init__.py
383-384: Audit URL open for permitted schemes. Allowing use of file: or custom schemes is often unexpected.
(S310)
387-388: Audit URL open for permitted schemes. Allowing use of file: or custom schemes is often unexpected.
(S310)
390-390: Audit URL open for permitted schemes. Allowing use of file: or custom schemes is often unexpected.
(S310)
⏰ 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). (5)
- GitHub Check: run-feature-tests-pg (15)
- GitHub Check: run-feature-tests-pg (17)
- GitHub Check: run-feature-tests-pg (14)
- GitHub Check: run-feature-tests-pg (16)
- GitHub Check: run-feature-tests-pg (18)
🔇 Additional comments (3)
web/pgadmin/misc/__init__.py (2)
12-12: LGTM!The certifi import is correctly placed and supports the SSL certificate handling enhancements in the
upgrade_checkfunction.
375-384: LGTM!The SSL context handling for Python 3.13+ correctly uses
ssl.create_default_context()with the appropriate CA file, falling back to certifi when the configured CA file is unavailable.requirements.txt (1)
22-22: Verify certifi package is free from security vulnerabilities.Version 2025.11.12 exists on PyPI. Manually verify this version against the PyPA Advisory Database (https://github.com/pypa/advisory-database) and CVE databases (NVD) to ensure it is free from known vulnerabilities.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.