openai-chat-app-quickstart icon indicating copy to clipboard operation
openai-chat-app-quickstart copied to clipboard

Enhanced Dependency Management for Cross-Platform Compatibility.

Open huntingcodes-001 opened this issue 1 year ago • 7 comments

Purpose

This Pull Request addresses compatibility issues with the installation of dependencies on different operating systems. It modifies the requirements.txt file to ensure seamless installation for both Linux and Windows environments, allowing for better usability across diverse systems.

Fixes for issue #160

Does this introduce a breaking change?

[ ] Yes
[x] No

Pull Request Type

What kind of change does this Pull Request introduce?

[ ] Bugfix
[x] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Documentation content changes
[ ] Other... Please describe:

How to Test

git clone https://github.com/huntingcodes-001/openai-chat-app-quickstart.git
cd openai-chat-app-quickstart
pip install -r requirements.txt
  • Test the code
1. Run the application using the command: `python app.py`
2. Verify that all features work as expected on both Windows and Linux environments.
3. Check for successful installation of all required dependencies without errors.

What to Check

Verify that the following are valid -All dependencies are installed without issues on both Windows and Linux systems. -The application runs smoothly and all functionalities are intact after the changes.

Other Information

Ensure that the development environment reflects the changes made in this PR. It may be helpful to run the tests on both operating systems to verify compatibility.

huntingcodes-001 avatar Oct 08 '24 18:10 huntingcodes-001

Thanks @huntingcodes-001 for the PR. The requirements.txt file is generated by this command: pip-compile --output-file=requirements.txt pyproject.toml

Will that retain the marker for the windows OS, or will it override when we run -U? I thought it would override.

pamelafox avatar Oct 08 '24 19:10 pamelafox

The other option:

requirements.in could bring in just uvicorn, not uvicorn[standard], and then bring in uvloop with the environment marker you have in this PR.

Here's what standard brings in: https://github.com/encode/uvicorn/blob/6ffaaf7c2f78274889da1572832dd307a8b117d3/pyproject.toml#L39

pamelafox avatar Oct 08 '24 19:10 pamelafox

You're correct that using pip-compile to regenerate the requirements.txt file will override any manual modifications, including the OS markers.

To ensure that the requirements are preserved for both Windows and Linux, we can consider adding a comment in the requirements.in file, indicating that the OS-specific installations should remain intact. Alternatively, we could explore using separate requirement files for different platforms if that fits better with our workflow.

huntingcodes-001 avatar Oct 08 '24 19:10 huntingcodes-001

Bringing in just uvicorn in requirements.in and specifying uvloop with an environment marker sounds like a solid approach. This would allow us to keep the installation lean while still accommodating the platform-specific needs.

huntingcodes-001 avatar Oct 08 '24 19:10 huntingcodes-001

Okay, so if we bring in just uvicorn, not uvicorn[standard], plus uvloop with a marker, we also need to decide whether to bring in the other packages from [standard]:

  • "colorama>=0.4;sys_platform == 'win32'": This just adds colored logs, and we don't use it now for Windows users, so we can keep not using it.
  • "httptools>=0.5.0": I think we want this, it seems like httptools is recommended from my read of the codebase.
  • "python-dotenv>=0.13": We already bring this in, so we don't need to bring it in again
  • "PyYAML>=5.1": I don't think we need this, since we're not using a YAML to configure uvicorn
  • "watchfiles>=0.13": This is helpful for using --reload, so we want this
  • "websockets>=10.4": I don't think we need this, since our app doesn't currently use websockets.

In conclusion, we want:

  • uvicorn
  • uvloop with marker
  • httptools
  • watchfiles

pamelafox avatar Oct 09 '24 13:10 pamelafox

Thanks for breaking that down!

Based on your suggestions, I’ll update the requirements.in to include the following:

uvicorn without [standard] uvloop with the platform-specific marker httptools (since it’s recommended) watchfiles (for --reload functionality) I'll leave out colorama, PyYAML, and websockets as discussed. This should keep the dependencies lean while ensuring the necessary functionality remains intact.

huntingcodes-001 avatar Oct 09 '24 16:10 huntingcodes-001

Sounds good, thanks!

pamelafox avatar Oct 09 '24 16:10 pamelafox

I just merged the fix that I suggested, so am closing this, thanks for sending the PR in!

pamelafox avatar Oct 25 '24 16:10 pamelafox