-
Notifications
You must be signed in to change notification settings - Fork 400
Store passthrough headers config in database #736
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
MohanLaksh
left a 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.
PR TEST SUMMARY:
make serve - PASS
make test - PASS
- Coverage 80% === 1629 passed, 10 skipped, 60 warnings in 70.13s (0:01:10) ===
make autoflake isort black flake8 - PASS
- PASS no errors
make pylint - PASS
- Your code has been rated at 10.00/10 (previous run: 10.00/10, +0.00)
make smoketest - PASS
- ✅ Smoketest passed!
make doctest - FAIL - NEEDS A FIX
- 53% Coverage - 1 failed, 468 passed, 6 skipped, 1 warning in 17.66s
- FAILED mcpgateway/utils/passthrough_headers.py::passthrough_headers.get_passthrough_headers
|
Fixed failing doctest |
Signed-off-by: Madhav Kandukuri <madhav165@gmail.com>
Signed-off-by: Madhav Kandukuri <madhav165@gmail.com>
Signed-off-by: Madhav Kandukuri <madhav165@gmail.com>
Signed-off-by: Madhav Kandukuri <madhav165@gmail.com>
crivetimihai
left a 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.
PR #736 Review Complete ✅
This PR adds functionality to persist passthrough headers configuration in the database, allowing global configuration management through the admin API endpoints.
Rebase Status: Successfully rebased on main (dropped 1 duplicate commit)
Test Results:
- ✅ Doctests: 469 passed, 6 skipped
- ✅ Unit tests: 1646 passed, 11 skipped (80% coverage)
- ✅ Linters:
- flake8: Clean
- pylint: 10.00/10
- black/isort: No formatting issues
Key Changes:
- Added database persistence for global passthrough headers configuration
- New admin API endpoints for managing passthrough headers (GET/PUT /admin/config/passthrough-headers)
- Initialization of global config on app startup when header passthrough is enabled
- Added PassthroughHeadersError exception class for better error handling
- Enhanced tests for the new functionality
The PR is ready to merge - all tests pass and code quality checks are clean.
445b557 to
38a9cde
Compare
* Store passthrough config in db Signed-off-by: Madhav Kandukuri <madhav165@gmail.com> * Fix failing test Signed-off-by: Madhav Kandukuri <madhav165@gmail.com> * Add tests and fix doctest Signed-off-by: Madhav Kandukuri <madhav165@gmail.com> * Fix test Signed-off-by: Madhav Kandukuri <madhav165@gmail.com> * Fix doctest Signed-off-by: Madhav Kandukuri <madhav165@gmail.com>
* Store passthrough config in db Signed-off-by: Madhav Kandukuri <madhav165@gmail.com> * Fix failing test Signed-off-by: Madhav Kandukuri <madhav165@gmail.com> * Add tests and fix doctest Signed-off-by: Madhav Kandukuri <madhav165@gmail.com> * Fix test Signed-off-by: Madhav Kandukuri <madhav165@gmail.com> * Fix doctest Signed-off-by: Madhav Kandukuri <madhav165@gmail.com>
* Store passthrough config in db Signed-off-by: Madhav Kandukuri <madhav165@gmail.com> * Fix failing test Signed-off-by: Madhav Kandukuri <madhav165@gmail.com> * Add tests and fix doctest Signed-off-by: Madhav Kandukuri <madhav165@gmail.com> * Fix test Signed-off-by: Madhav Kandukuri <madhav165@gmail.com> * Fix doctest Signed-off-by: Madhav Kandukuri <madhav165@gmail.com>
🐛 Bug-fix PR
📌 Summary
The global passthrough headers were not being stored to the database even if the passthrough was enabled. This adds a function to store them during app initialization.
💡 Fix Description
set_global_passthrough_headersfunction in passthrough_headers to store passthrough_headers to databaselifespanfunction🧪 Verification
make lintmake testmake coverage✅ Checklist
make black isort pre-commit)