-
Notifications
You must be signed in to change notification settings - Fork 84
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
fixed python-multipart and multipart #1619
Conversation
WalkthroughThe pull request introduces several modifications across multiple files. In Changes
Possibly related PRs
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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
🧹 Outside diff range and nitpick comments (2)
requirements/prod.txt (1)
36-37
: Consider specifying upper version boundsThe version constraints for RestrictedPython and AccessControl have been relaxed to use >= without upper bounds. This could lead to compatibility issues if future versions introduce breaking changes.
Consider using version ranges instead:
-RestrictedPython>=7.3 -AccessControl>=7.2 +RestrictedPython>=7.3,<8.0 +AccessControl>=7.2,<8.0tests/integration_test/history_client_test.py (1)
1254-1254
: Improved precision in wordcloud parametersThe change from integer to floating-point numbers (
u_bound=1.0
andl_bound=0.0
) enhances the precision of the wordcloud generation parameters.Consider documenting the valid ranges for these parameters in the API documentation to help users understand the expected values and their impact on wordcloud generation.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
kairon/api/app/routers/bot/data.py
(1 hunks)metadata/llm_metadata.yml
(1 hunks)requirements/prod.txt
(3 hunks)tests/integration_test/history_client_test.py
(1 hunks)
🔇 Additional comments (4)
requirements/prod.txt (2)
71-71
: Verify python-multipart version compatibility with FastAPI
The addition of python-multipart>=0.0.18
aligns with the PR objective. However, we should verify compatibility with FastAPI 0.115.6.
✅ Verification successful
Python-multipart version is compatible with FastAPI
The specified version python-multipart>=0.0.18
is compatible with FastAPI 0.115.6. FastAPI's own dependency specification requires python-multipart>=0.0.7
, so our version constraint of 0.0.18 or higher satisfies this requirement. The package is used for handling form data and file uploads in FastAPI applications.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check FastAPI's requirements.txt for python-multipart version constraints
curl -s https://raw.githubusercontent.com/tiangolo/fastapi/0.115.6/requirements.txt | grep "python-multipart"
Length of output: 109
Script:
#!/bin/bash
# Let's check FastAPI's setup.py or pyproject.toml for dependencies
curl -s https://raw.githubusercontent.com/tiangolo/fastapi/0.115.6/pyproject.toml | grep -A 5 -B 5 "multipart"
Length of output: 844
Script:
#!/bin/bash
# Let's check FastAPI's documentation for python-multipart requirements
curl -s https://raw.githubusercontent.com/tiangolo/fastapi/0.115.6/docs/en/docs/tutorial/request-forms.md | grep -A 5 -B 5 "multipart"
Length of output: 1125
Script:
#!/bin/bash
# Let's check if python-multipart is used in the codebase and how
rg "python-multipart|multipart" -A 3 -B 3
Length of output: 1010
3-4
: Verify FastAPI and Uvicorn version compatibility
The upgrade to FastAPI 0.115.6 and Uvicorn 0.32.0 represents significant version jumps. Let's verify there are no breaking changes.
✅ Verification successful
Let me gather more information about the FastAPI usage in the codebase to better assess the compatibility.
FastAPI and Uvicorn version upgrade appears safe
The codebase uses standard FastAPI features (routing, middleware, responses, security) and basic Uvicorn configuration. The changes in Uvicorn 0.32.0 are mainly additions (Python 3.13 support and request limit warning) without breaking changes. FastAPI 0.115.6 is a stable version, and the codebase doesn't use any deprecated features or patterns that would be affected by the upgrade.
- The main FastAPI usage patterns found:
- Standard middleware (CORS, GZip)
- Basic routing and API endpoints
- Common response types (JSONResponse, FileResponse)
- Standard security features (OAuth2, dependencies)
- Uvicorn is used with default configuration for ASGI server deployment
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for potential breaking changes in FastAPI changelog
curl -s https://raw.githubusercontent.com/tiangolo/fastapi/master/CHANGELOG.md | grep -A 10 "0.115.6"
# Check for potential breaking changes in Uvicorn changelog
curl -s https://raw.githubusercontent.com/encode/uvicorn/master/CHANGELOG.md | grep -A 10 "0.32.0"
Length of output: 364
Script:
#!/bin/bash
# Check FastAPI usage patterns in the codebase
ast-grep --pattern 'from fastapi import $$$'
# Check for FastAPI app initialization
ast-grep --pattern 'FastAPI($$$)'
# Look for uvicorn configuration or startup code
rg "uvicorn\." -A 2
Length of output: 8907
metadata/llm_metadata.yml (1)
21-21
: Breaking Change: Verify impact of removing gpt-3.5-turbo-instruct model
The removal of "gpt-3.5-turbo-instruct" from the allowed models could break existing configurations. We should verify if any existing configurations use this model.
✅ Verification successful
No breaking changes detected - model only used in test data
The removed model "gpt-3.5-turbo-instruct" is only referenced in a test file (tests/testing_data/llm_metadata.yml
) and not in any production configuration files. The change in the actual metadata schema (metadata/llm_metadata.yml
) is safe to proceed with.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for usage of the removed model in configuration files
rg -l "gpt-3.5-turbo-instruct" --type yaml --type json
Length of output: 90
Script:
#!/bin/bash
# Check the content of the test file to understand the usage context
cat tests/testing_data/llm_metadata.yml
Length of output: 5584
kairon/api/app/routers/bot/data.py (1)
11-16
: LGTM: Import organization improves readability
The reorganization of imports and explicit import of AppException improves code organization and maintainability.
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.
approved
Summary by CodeRabbit
Release Notes
New Features
Changes
fastapi
,uvicorn
, and addedpython-multipart
.Bug Fixes
Tests