-
Notifications
You must be signed in to change notification settings - Fork 592
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
remove itsdangerous pin #5403
remove itsdangerous pin #5403
Conversation
WalkthroughThe pull request introduces a new package dependency, Changes
Possibly related PRs
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
Documentation and Community
|
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.
it'd be nice to have a plan to keep up to date with their (what appears to be annual) releases, but this'll work for now!
915c7b8
to
3439365
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: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
requirements/test.txt
(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: test / test-python (ubuntu-latest-m, 3.10)
- GitHub Check: test / test-app
- GitHub Check: e2e / test-e2e
- GitHub Check: build / build
- GitHub Check: build
🔇 Additional comments (2)
requirements/test.txt (2)
11-11
: LGTM!The formatting change for twine's version constraint maintains consistency without affecting the version requirement.
1-1
: Verify compatibility with open3d 0.19.0.The change from
>=0.16.0
to>0.16.0
makes the lower bound exclusive. While this allows compatibility with version 0.19.0, we should verify there are no breaking changes in the intermediate versions.Let's check the package compatibility:
✅ Verification successful
Version constraint
open3d>0.16.0
is appropriateThe change is safe because:
- Version 0.19.0 exists and is supported
- The codebase uses lazy imports with package verification
- No version-specific API dependencies were found
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check open3d version compatibility and its dependencies # Get latest version and release date curl -s https://pypi.org/pypi/open3d/json | jq -r '.releases | keys_unsorted[-1] as $latest | [.[$latest][0].upload_time_iso_8601, $latest] | @tsv' # Check for any Flask dependencies in open3d curl -s https://pypi.org/pypi/open3d/0.19.0/json | jq -r '.info.requires_dist | .[] | select(contains("Flask"))'Length of output: 300
Script:
#!/bin/bash # Try alternative PyPI API endpoint and check repository # Check if open3d 0.19.0 exists curl -s -I "https://files.pythonhosted.org/packages/source/o/open3d/open3d-0.19.0.tar.gz" | head -n 1 # Search for version-specific requirements or configuration fd -e txt -e ini -e cfg -e yaml -e yml . | xargs grep -l "open3d" # Check if there are any test files using open3d rg "import.*open3d" -lLength of output: 460
Script:
#!/bin/bash # Check actual usage of open3d in source files rg "import.*open3d" -A 5 tests/unittests/utils3d_tests.py fiftyone/utils/utils3d.py fiftyone/utils/kitti.py # Check available versions on PyPI curl -s https://pypi.org/pypi/open3d/json | jq -r '.releases | keys[]' | sort -V | tail -n 5Length of output: 1155
requirements/test.txt
Outdated
open3d>=0.16.0 | ||
itsdangerous==2.0.1 | ||
open3d>0.16.0 | ||
itsdangerous==2.1.2 |
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.
💡 Codebase verification
Version 2.1.2 of itsdangerous is incompatible with Flask
Flask requires itsdangerous>=2.2, but the requirements file specifies version 2.1.2. Please upgrade itsdangerous to at least version 2.2 to ensure compatibility.
🔗 Analysis chain
Verify Flask compatibility with itsdangerous 2.1.2.
The version update to 2.1.2 should resolve the build issues with Flask dependencies. Let's verify the compatibility.
Run this script to check version compatibility:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check itsdangerous version compatibility with Flask
# Get Flask's dependency on itsdangerous
curl -s https://pypi.org/pypi/Flask/json | jq -r '.info.requires_dist | .[] | select(contains("itsdangerous"))'
# Check for any known issues with itsdangerous 2.1.2
gh api graphql -f query='
{
securityVulnerabilities(first: 5, ecosystem: PIP, package: "itsdangerous", package: "Flask") {
nodes {
advisory {
summary
severity
publishedAt
}
vulnerableVersionRange
firstPatchedVersion {
identifier
}
}
}
}'
Length of output: 733
Script:
#!/bin/bash
# Check for security vulnerabilities in itsdangerous
gh api graphql -f query='
{
securityVulnerabilities(first: 5, ecosystem: PIP, package: "itsdangerous") {
nodes {
advisory {
summary
severity
publishedAt
}
vulnerableVersionRange
firstPatchedVersion {
identifier
}
}
}
}'
Length of output: 345
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: 2
♻️ Duplicate comments (1)
requirements/test.txt (1)
3-3
:⚠️ Potential issueUpgrade itsdangerous to version >=2.2.
Flask requires itsdangerous>=2.2, but version 2.1.2 is specified. This will cause compatibility issues.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
requirements/test.txt
(2 hunks)
🧰 Additional context used
🪛 GitHub Actions: Build docs
requirements/test.txt
[error] 2-2: Dependency conflict between ipywidgets and open3d. open3d requires ipywidgets>=8.0.4, but there is a conflicting requirement for ipywidgets<8 and >=7.5
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: test / test-python (ubuntu-latest-m, 3.11)
- GitHub Check: test / test-python (ubuntu-latest-m, 3.10)
- GitHub Check: test / test-python (ubuntu-latest-m, 3.9)
- GitHub Check: e2e / test-e2e
requirements/test.txt
Outdated
open3d>=0.16.0 | ||
itsdangerous==2.0.1 | ||
boto3==1.17.36 | ||
open3d>0.16.0 |
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.
Resolve dependency conflict with ipywidgets.
The pipeline failure indicates that open3d requires ipywidgets>=8.0.4, but there's a conflicting requirement for ipywidgets<8. Consider one of these solutions:
- Pin open3d to a version that's compatible with ipywidgets<8
- Update the ipywidgets constraint to allow version 8
🧰 Tools
🪛 GitHub Actions: Build docs
[error] 2-2: Dependency conflict between ipywidgets and open3d. open3d requires ipywidgets>=8.0.4, but there is a conflicting requirement for ipywidgets<8 and >=7.5
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: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
requirements/extras.txt
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: test / test-python (ubuntu-latest-m, 3.11)
- GitHub Check: test / test-python (ubuntu-latest-m, 3.10)
- GitHub Check: e2e / test-e2e
- GitHub Check: test / test-app
- GitHub Check: build
🔇 Additional comments (1)
requirements/extras.txt (1)
4-4
: Verify alignment with PR objectives.This change appears to be unrelated to the main PR objective of fixing Flask dependencies and removing the itsdangerous pin. Please clarify if this modification is necessary for the current PR scope.
Let's check if there are any dependency relationships between ipywidgets and the Flask-related packages:
@@ -1,7 +1,7 @@ | |||
google-api-python-client>=1.6.5 | |||
google-cloud-storage>=1.36 | |||
httplib2<=0.15 | |||
ipywidgets>=7.5,<8 | |||
ipywidgets>=7.5 |
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.
💡 Codebase verification
Maintain version constraint for ipywidgets
The removal of the upper version bound (<8
) could lead to compatibility issues as ipywidgets has undergone a major version change (current latest is 8.1.5). Since major versions often introduce breaking changes and the package has tightly coupled dependencies, it's recommended to maintain version constraints:
- Consider using
ipywidgets>=7.5,<8
to ensure stability - If v8 support is needed, verify compatibility with your usage first
🔗 Analysis chain
Verify compatibility with newer ipywidgets versions.
Removing the upper version bound (<8
) could potentially introduce compatibility issues with newer versions of ipywidgets. While this change allows for more flexibility, it may need additional testing to ensure compatibility with the rest of the ecosystem.
Let's check for any known compatibility issues or breaking changes in newer versions:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for major version changes and dependencies of ipywidgets
# Get the latest version and its dependencies
curl -s https://pypi.org/pypi/ipywidgets/json | jq -r '.releases | keys[]' | sort -V | tail -n 1
curl -s https://pypi.org/pypi/ipywidgets/json | jq '.info.requires_dist'
# Check if this package is a dependency of other packages in the project
rg -l "ipywidgets" requirements/
Length of output: 531
* pin open3d * more than 16, less than 19 * remove itsdangerous
* pin open3d * more than 16, less than 19 * remove itsdangerous
open3d 0.19.0 has updated flask dependencies that breaks build
Summary by CodeRabbit
awscli==1.37.2
.boto3
to1.36.2
.itsdangerous
.twine
version constraint for consistency.ipywidgets
to allow versions 7.5 and above.