Move --no-binary=lxml,xmlsec into the Dockerfile
#8361
Conversation
The intention was for this option to only be set in the production requirements file, but due to how pip-compile-multi works, it ends up leaking into all other files too. Moving it to the Dockerfile fixes this.
WalkthroughThe changes involve modifications to several requirements files related to Python package management. Key alterations include the removal of Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant App
participant PackageManager
User->>App: Request to install packages
App->>PackageManager: Process installation
PackageManager->>PackageManager: Check dependencies
PackageManager->>PackageManager: Install dependencies (binary if available)
PackageManager-->>App: Return installation status
App-->>User: Confirm package installation
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 as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (7)
- Dockerfile (1 hunks)
- cvat/requirements/all.txt (1 hunks)
- cvat/requirements/base.txt (2 hunks)
- cvat/requirements/development.txt (1 hunks)
- cvat/requirements/production.in (1 hunks)
- cvat/requirements/production.txt (1 hunks)
- cvat/requirements/testing.txt (1 hunks)
Files skipped from review due to trivial changes (4)
- cvat/requirements/base.txt
- cvat/requirements/development.txt
- cvat/requirements/production.txt
- cvat/requirements/testing.txt
Additional comments not posted (3)
cvat/requirements/production.in (1)
Line range hint
1-7: Verify the impact of removing--no-binarydirective.The removal of the
--no-binarydirective forlxmlandxmlsecsimplifies the dependency declaration. Ensure that this change does not affect the functionality of the application, especially in production environments where these libraries are critical.Run the following script to check for potential issues:
cvat/requirements/all.txt (1)
Line range hint
1-13: Verify the impact of removing--no-binarydirective.The removal of the
--no-binarydirective forlxmlandxmlsecsimplifies the dependency declaration. Ensure that this change does not affect the functionality of the application, especially in production environments where these libraries are critical.Run the following script to check for potential issues:
Dockerfile (1)
83-84: LGTM!The addition of the
--no-binarydirective ensures thatlxmlandxmlsecare built from source. This change addresses issues related to binary distributions of these libraries.The code changes are approved.
|
@Marishka17 What do you think? |
|
@SpecLad, I liked when |
|
Alright, I tested it. |
The intention was for this option to only be set in the production requirements file, but due to how pip-compile-multi works, it ends up leaking into all other files too. Moving it to the Dockerfile fixes this.
Motivation and context
The intention was for this option to only be set in the production requirements file, but due to how pip-compile-multi works, it ends up leaking into all other files too. Moving it to the Dockerfile fixes this.
How has this been tested?
I re-ran the Docker build to make sure lxml and xmlsec are actually built from source.
Checklist
developbranch[ ] I have created a changelog fragment[ ] I have updated the documentation accordingly[ ] I have added tests to cover my changes[ ] I have linked related issues (see GitHub docs)[ ] I have increased versions of npm packages if it is necessary(cvat-canvas,
cvat-core,
cvat-data and
cvat-ui)
License
Feel free to contact the maintainers if that's a concern.
Summary by CodeRabbit
New Features
lxmlandxmlsec, improving installation flexibility.Bug Fixes
dj-rest-authto ensure correct package resolution during installation.Chores