-
Notifications
You must be signed in to change notification settings - Fork 51
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
Improvement: Requirements installation in venv #686
Conversation
…buntu > 23.xx, cleaned up checks for better understanding Signed-off-by: jdsika <carlo.van-driesten@vdl.digital>
Signed-off-by: jdsika <carlo.van-driesten@vdl.digital>
@vkresch please a quick review here |
Signed-off-by: jdsika <carlo.van-driesten@vdl.digital>
Hi! Can we revert this? It no longer works in a container.
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? |
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... |
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. |
…virtual-venv Revert "Improvement: Requirements installation in venv (tezos-reward-distributor-organization#686)"
yes, please remove the |
@jdsika I am removing the in_venv check in this PR #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. |
As long as the program shuts down gracefully if the packages are missing. |
Work effort: 4h