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

WIP: Implement python virtual environment #263

Closed
wants to merge 4 commits into from

Conversation

aroundthfur
Copy link
Contributor

@aroundthfur aroundthfur commented Oct 17, 2021

  • added venv instead of using system env pip
  • adjusted the unit file to startup the app from the venv
  • created a wrapper script to call the app from the venv
  • extended cleanup for venv and additional scripts
  • refactored the is_running() function to find the process now that it
    is called from a venv

I tested this on Fedora 35. It worked fine (TM).
Please test on other platforms before merging. I am especially not sure how this change reflects on SNAP and other packaging systems.

  * added venv instead of using system env pip
  * adjusted the unit file to startup the app from the venv
  * created a wrapper script to call the app from the venv
  * extended cleanup for venv and additional scripts
  * refactored the is_running() function to find the process now that it
    is called from a venv
@aroundthfur aroundthfur changed the title Implement python virtual environment WIP: Implement python virtual environment Oct 22, 2021
@aroundthfur
Copy link
Contributor Author

Please do not merge yet!

I am experiencing random SIGKILL on the process. Not sure why yet, it needs to be debugged more.

@bobslept
Copy link
Contributor

Could it be that power-profiles-daemon had something to do with it at the time of testing this?

@AdnanHodzic
Copy link
Owner

Could it be that power-profiles-daemon had something to do with it at the time of testing this?

@bobslept yep, that exactly was the reason as it was confirmed to me by @aroundthfur in private. Btw, whenever you have time /you're able feel free to push final updates to this PR.

@bobslept
Copy link
Contributor

What would be the correct way of doing this. Merge his branch from the fork locally, work on it and push it as new PR and mention this PR in the new one?

I can do it like that if you want. Not sure if it is the right way.

@AdnanHodzic
Copy link
Owner

What would be the correct way of doing this. Merge his branch from the fork locally, work on it and push it as new PR and mention this PR in the new one?

I can do it like that if you want. Not sure if it is the right way.

Please do this. As I also got @aroundthfur consent to do this (make new PR on top of his changes) in the end I'll just credit you both.

@AdnanHodzic
Copy link
Owner

Changes live as part of v1.9.0 release.

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.

3 participants