Watcher3
Watcher3 copied to clipboard
Make Watcher3 pip installable and unvendor libs.
This begins the work on making Watcher3 pip installable. It opts for setuptools instead of Poetry. Steps to install:
git clone https://github.com/barbequesauce/watcher3
cd watcher3
# optionally create a virtual environment and activate it here
pip install m2r # required because cherrypyscheduler fails to install without it
pip install . # or pip install -e . for an editable install for development
The above steps are currently functional. I'd like to continue development to make the project fully pip-installable through a command like pip install watcher3@git+https://github.com/barbequesauce/watcher3@master or if you create a pypi package simply pip install watcher3. This would completely remove the requirement to git clone the repo and would give the added benefit of being able to upgrade Watcher3 with pip install --upgrade watcher3.
@barbequesauce Let me know how you feel about this PR.
This PR beings the implementation of a system to meet the goal of #161 using setuptools instead. Once pip-installable you could use pipx install watcher3 to make an isolated build of Watcher3. Then to run it you would pipx run watcher3.
I think you shouldn't unvendor some libs which were changed in Watcher3, check the changes: https://github.com/barbequesauce/Watcher3/blob/master/lib/mods.txt
I think PTN got many changes, I would like to replace it with something else as it isn't maintained, so probably it should never be unvendored.
The changes on cheroot and transmissionrpc may not be needed if they are related to some bug which is fixed in a newer version, but it should be checked before unvendoring them
About PTN, another way to unbundle it would be creating a fork and pushing our changes there.
@scambra like in #205 I'm not yet addressing the issues in mods.txt, mainly just to provide a functional proof of concept before it makes its way into master. Some of the changes may be obsolete. For example I'll be using my fork of transmission-rpc which uses requests instead of urllib. Also if a fix is simple enough to monkey-patch, I'd rather install the upstream version and monkey-patch locally. If the changes are significant enough then I think forking it with the changes is the best way to go. I'm generally against vendoring libs as they tend to get ignored and never get any TLC or QoL improvements or even security fixes. Also when people vendor libs their changes frequently never make their way upstream to improve the code base for others. If you're contributing upstream then that's more people working on those repositories as well, improving the whole ecosystem. The point is I do plan on addressing them before the code gets merged into master, but probably not before going to develop.
@scambra oh and I forgot to say, thanks for pointing out the mods.txt, while I had already seen it, its always great to see others looking out for potential issues, as we can't catch everything. I appreciate it.
@labrys agree with you, I don't like vendoring, I had to upgrade some lib and it's a pain. Although I think vendoring PTN is ok as it's not maintained, and probably is better to replace it with guessit or something else later. Although forking it with our changes is probably better than vendoring.
If the changes on transmission or cheroot are not needed anymore, it's great, otherwise we should send a PR, and fork or monkey patch them until fix is merged into upstream.
@scambra @labrys did either of you had further thoughts on PTN? Other than that this patch has been running nicely on my system since PR submission, I'm ok to merge it other than the PTN question... transmission changes dont seem to be needed (tx is my torrent client of choice) and cheroot hasn't shown any issues.
@barbequesauce In my opinion the easiest path forward is forking PTN. While replacing it in the future would probably advisable, there's no reason to delay this for a significant change like that.
I'm thinking the same @labrys - I want to give @scambra a chance to weigh in as I'd say they know the code base better than me...
I agree, I wanted to fork PTN and push the changes, but I had no time yesterday
@scambra take a look at #211 - thoughts?
@labrys Can you change parse-torrent-name with parse-torrent-title? Then we should work in a branch to adapt our code to the new fields in PTN