tezos-reward-distributor-organization / tezos-reward-distributor

Tezos Reward Distributor (TRD): A reward distribution software for tezos bakers.
https://tezos-reward-distributor-organization.github.io/tezos-reward-distributor/
GNU General Public License v3.0
87 stars 51 forks source link

Improvement: Requirements installation in venv #686

Closed jdsika closed 9 months ago

jdsika commented 9 months ago

Work effort: 4h

jdsika commented 9 months ago

@vkresch please a quick review here

nicolasochem commented 8 months ago

Hi! Can we revert this? It no longer works in a container.

│ Checking python version ...                                                                                                                                                                │
│                                                                                                                                                                                            │
│ ... installed Python version 3.10 is greater then minimum required version 3.7. OK!                                                                                                        │
│                                                                                                                                                                                            │
│ Please make sure to activate a virtual environment for python due to breaking changes in Ubutu >= 23.XX:                                                                                   │
│ https://packaging.python.org/en/latest/guides/installing-using-pip-and-virtual-environments/                                                                                               │
│                                                                                               

When running containerized, there is little reason to use a venv... Can you make this check more specific? Like, check for the exact reason you are making this mandatory? What's the ubuntu 23 thing?

jdsika commented 8 months ago

After Ubuntu 23 it is considered a bad practice to install python packages system wide as it may interfere with the system functionality. You cannot use pip install anymore without an error. I actually think it makes sense and haven't thought about it before.

The message obviously does not tell the right story... https://fostips.com/pip-install-error-ubuntu-2304/?amp=1

lukeisontheroad commented 7 months ago

For me, this broke the dockerized version, it's also not running with the build pushed yesterday and since the protocol EOL was hard coded in the constants I can't get it running anymore.

nicolasochem commented 7 months ago

yes, please remove the in_venv() check from main branch @jdsika

vkresch commented 7 months ago

@jdsika I am removing the in_venv check in this PR https://github.com/tezos-reward-distributor-organization/tezos-reward-distributor/pull/690. Let me know if it's fine. Imo the user is responsible for setting up his environment for TRD correctly. We might actually also consider removing the requirements_installed check since we provide the requirements.txt file which lists the packages the user should have installed before running TRD.

jdsika commented 7 months ago

As long as the program shuts down gracefully if the packages are missing.