Sonarr and Radarr fixes #3163
Sonarr and Radarr fixes #3163
Conversation
spk/sonarr/src/service-setup.sh
Outdated
| # But there is a problem: | ||
| # On DSM5 this is: PKG/var/.config/ | ||
| # On DSM6 this is: PKG/.config/ | ||
| # So we have to do upgrade magic |
There was a problem hiding this comment.
I do not like such complexity which is error prone and requires intensive testing.
Isn't it possible to set HOME to PKG/var/ in process environment at Sonarr startup with env for instance ? it is already done for other packages...
spk/radarr/src/service-setup.sh
Outdated
| # But there is a problem: | ||
| # On DSM5 this is: PKG/var/.config/ | ||
| # On DSM6 this is: PKG/.config/ | ||
| # So we have to do upgrade magic |
There was a problem hiding this comment.
Some request as for Sonarr. Please keep it simple and change HOME at process startup.
There was a problem hiding this comment.
Ah, I didn't know that was possible. I hope Sonarr listens to that variable. Will test.
Wonder why that approach wasn't taken before when they did this overhaul: 5a0b6da
And now finally add them to 'sc-download'
Safihre
force-pushed
the
sonarr-radarr-fixes
branch
2 times, most recently
from
4cf5cd1 to
d3b06d3
Compare
Safihre
force-pushed
the
sonarr-radarr-fixes
branch
from
d3b06d3 to
c8e1db2
Compare
|
@ymartin59 Thanks for the hint. The |
|
Checked on DSM 5.2 and DSM6.1:
💯 💯💯 |
| if [ -d "${CONFIG_DIR}" ]; then | ||
| echo "Moving ${CONFIG_DIR} to ${INST_VAR}" >> ${INST_LOG} | ||
| mv ${CONFIG_DIR} ${INST_VAR}/.config | ||
| if [ -d "${LEGACY_CONFIG_DIR}" ]; then |
There was a problem hiding this comment.
@Safihre I still do not understand why there is some code to handle .config "move" whereas there is no longer "move" as its location has never changed between published package for DSM 5 in synocommunity repository.
May you explain ?
There was a problem hiding this comment.
The people that have now installed it on DSM 6 (which has the different home directory) have it in that directory. So we need to take care of it.
There was a problem hiding this comment.
OK so I guessed right but that is not my point of view... beta/test packages are "as is"... that is why I avoid to publish when it is not really ready. It is far enough to "support" packages published in repository, not to add complexity to support/track "not ready" packages. Imagine I discard that useless code and let you explain users how to upgrade manually ?
There was a problem hiding this comment.
I really think you are "pushing" too fast content, before it has been at least reviewed or tested by someone else. You merged a lot of PRs bulk... but be sure I will not publish any of them without reviewing again, and testing locally.
There was a problem hiding this comment.
OK so I guessed right but that is not my point of view... beta/test packages are "as is"... that is why I avoid to publish when it is not really ready. It is far enough to "support" packages published in repository, not to add complexity to support/track "not ready" packages. Imagine I discard that useless code and let you explain users how to upgrade manually ?
They will get the update via DSM package center automatically, how do you want to instruct them to upgrade properly? Just the sonarr package has more than 400+ downloads among the different versions I released.
There was a problem hiding this comment.
Your comments here disappoint me. You seem to suggest that it was wrong of me to supply testing packages. That those testing packages were not really ready and that if their setup brakes because you remove this codd, it's kind of my fault.
While in reality, without testing packages we wouldn't be able to identify problems with this and other packages.
And yes, I merge stuff, because I am tired of my code just being left there in those PRs. Yes of course I want you to review and test them, but when will you actually do that?
The code just sits there, after I have spend so much time on it.
I have spend countless hours already since last November, the last 2 weeks even fulltime! And now you make it sound like I just add code without careful consideration. While in fact it was all my careful testing on all platforms that revealed so many previously unknown problems.
I respect your position to handle the publishing and I am glad you do it. So I hope you will also do the job of inspecting and testing my code, not just let it sit there for weeks waiting for.. Well I am not even sure what everything is constantly waiting for here.
|
@Safihre You're right, I am aware of and I really appreciate your investment here. My point is: even if you feel confident in your code and testing - you should wait for (at least) a first review to have chance to eliminate "first level" mistakes and get some hints (like the HOME variable trick). I am OK you build and publish test packages after such a first review. A pending PR is not "lost work"... I even plan to resurrect interested closed PR that have never been merged like squidguard. I have not as much bandwidth as you have to work on it, but we did great progress together... and I am relieved that my PR can be reviewed too. Again, there is no need to hurry, even if PR is merged, package will not be published "faster". |
* Sonarr/Radarr - Keep legacy membership of sc-media And now finally add them to 'sc-download' * Sonarr/Radarr - Use generic set_unix_permissions * Sonarr/Radarr - Home directory handeling for DSM 5 and 6
Needs #3161, separated for clarity.
Uses the functions and changes there to make Sonarr/Radarr work on DSM6 but also 5.
Checklist
all-supportedcompleted successfully