Skip to content
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

Fix Dockerfile #210

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

guillaumelambert
Copy link

Requests is no more installed in python:3.11-slim-bullseye image.

Issue: #209

Requests is no more installed in python:3.11-slim-bullseye image.

Issue: megadose#209
@Pshepp
Copy link

Pshepp commented Dec 15, 2024

Seems cleaner and more robust to, instead of handling the install through the dockerfile, respect the projects design pattern and add ,"requests" to

install_requires=["termcolor","bs4","httpx","trio","tqdm","colorama"],

Tested this locally and it works well. The setup.py file is also used when installing the project on bare metal.

@guillaumelambert
Copy link
Author

Seems cleaner and more robust to, instead of handling the install through the dockerfile, respect the projects design pattern and add ,"requests" to

install_requires=["termcolor","bs4","httpx","trio","tqdm","colorama"],

Tested this locally and it works well. The setup.py file is also used when installing the project on bare metal.

Thanks for the feedback. Please Feel free to propose the related PR.
There is always a debate how should python dependecies be installed.
Both methods are perfectly acceptable IMHO and are really up to the maintainer.
I won't say using setup.py is cleaner or more robust.
It can encourage bad practices such a running pip install under root.
If it can be OK under certains circumstances or in a docker container, it is usually not on a production system.
When possible, I prefer to rely on system packages validated by the distribution maintainers.
At first glance, it may look more robust here but there are some cons.
setup.py is still supported but tends to be replaced by project.toml now
Note also the syntax python setup.py install works but is no more recommanded and has been superseded python -m pip install
Hope this helps

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants