airsonic / airsonic

:satellite: :cloud: :notes:Airsonic, a Free and Open Source community driven media server (fork of Subsonic and Libresonic)
https://airsonic.github.io
GNU General Public License v3.0
2.01k stars 238 forks source link

New beta release should inform users that war now embeds tomcat. #280

Closed issuemover631 closed 7 years ago

issuemover631 commented 7 years ago

Issue by hurricanehrndz Tuesday Mar 07, 2017 at 16:19 GMT Originally opened as https://github.com/Libresonic/libresonic/issues/278


Issue: https://github.com/Libresonic/libresonic/issues/142

Included the migration to springboot and many others. @biconou mentions that libresonic-main now produces a libresonic.war standalone package that embeds tomcat.

This breaks current deployments for anyone who was using a tomcat or jetty server, which is not a big issue, unless you fail to mention it in the release notes. The only information that I could find about the war was issue 142.

In the future I believe it would be valuable to add a note that mentions major architectural changes like the ones executed in the following merge https://github.com/Libresonic/libresonic/pull/176.

A note that is clear and concise and doesn't to obfuscate the massive changes by just mentioning one of the many architectural changes that occurred in that said pull request.

Thank you for your amazing work again.

issuemover631 commented 7 years ago

Comment by muff1nman Tuesday Mar 07, 2017 at 18:54 GMT


It should not break deployments on tomcat. If so it's a bug so please open a issue with details. The general idea is that while tomcat is bundled inside, spring boot should put it in a location such that it is ignored if it is run on an application server.

issuemover631 commented 7 years ago

Comment by hurricanehrndz Tuesday Mar 07, 2017 at 19:12 GMT


Oh, well it broke deployments with jetty. Thank you for the information.

On Mar 7, 2017 11:54 AM, "Andrew DeMaria" notifications@github.com wrote:

It should not break deployments on tomcat. If so it's a bug so please open a issue with details. The general idea is that while tomcat is bundled inside, spring boot should put it in a location such that it is ignored if it is run on an application server.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/Libresonic/libresonic/issues/278#issuecomment-284819896, or mute the thread https://github.com/notifications/unsubscribe-auth/AFiQzcp0InZI22nZ8BWAqi7dsrmxm1YJks5rjaf9gaJpZM4MVsdW .

issuemover631 commented 7 years ago

Comment by muff1nman Tuesday Mar 07, 2017 at 20:13 GMT


Oh I see. I'm not sure any of us tested the jetty case. Could you still open an issue on it?

issuemover631 commented 7 years ago

Comment by texneus Tuesday Mar 07, 2017 at 20:54 GMT


Could somebody elaborate on how embedding Tomcat works? Does this not lock the WAR to a specific platform (presumably a Linux distro)? Which one? How would somebody use this on Windows, FreeBSD, OSX, etc?

What about security? Is the embedded Tomcat secured (ex. https://www.upguard.com/articles/15-ways-to-secure-apache-tomcat-8 or http://security-24-7.com/hardening-guide-for-tomcat-8-on-redhat-6-5-64bit-edition/)? Is https enabled?

P.S. Thanks for all the hard work everyone! Been hanging in the background here since v5!

issuemover631 commented 7 years ago

Comment by muff1nman Tuesday Mar 07, 2017 at 21:17 GMT


Alright I give in folks, I'll take a to-do to explain this in more detail in the docs.

issuemover631 commented 7 years ago

Comment by hurricanehrndz Tuesday Mar 07, 2017 at 21:29 GMT


Yes, but more importantly major changes like this needs to be explicitl mentioned in release logs. It can just say "changed subsonic-boot to spring boot".

On Mar 7, 2017 2:17 PM, "Andrew DeMaria" notifications@github.com wrote:

Alright I give in folks, I'll take a to-do to explain this in more detail in the docs.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/Libresonic/libresonic/issues/278#issuecomment-284862120, or mute the thread https://github.com/notifications/unsubscribe-auth/AFiQzejXk_mIH2CeIPC73WQYEojW5Uk6ks5rjcl3gaJpZM4MVsdW .

issuemover631 commented 7 years ago

Comment by muff1nman Tuesday Mar 07, 2017 at 21:58 GMT


I disagree, the release logs are not meant to teach people about the underlying technology. Any necessary configuration guides will be found in the documentation folder

issuemover631 commented 7 years ago

Comment by hurricanehrndz Tuesday Mar 07, 2017 at 22:59 GMT


Nope but release logs should have important information that document massive architectural changes, such as no longer using library x, instead using y, or war now embeds tomcat. Package maintainers and builders usually depend heavily on release logs in order to update packages. This of course is just my opinion, if the libresonic team feels differently, I will totally respect your stance. I just believe that appropriately documenting releases or the project is necessary so that users are not reading through issues and merge request to try and figure what might have broken their otherwise working packages or container in my case.

Again, I'm really grateful for all your hard work and this issue is more of a suggestion in the hopes that it will improve the experience of all end user. Thanks again for your consideration.

issuemover631 commented 7 years ago

Comment by muff1nman Wednesday Mar 08, 2017 at 00:01 GMT


I just believe that appropriately documenting releases or the project is necessary so that users are not reading through issues and merge request to try and figure what might have broken their otherwise working packages or container in my case.

Give us the opportunity to help you and others in this case and open up an issue with what you are seeing. We might not be able to help right away, but especially with these beta releases, we need people to tell us what they are running into. A quick search through the existing issues to see if someone already has brought up your problem is great, but sometimes you've run into something we need to be aware of.

We definitely want to improve the end user experience, and along that route I'd rather address the actual issue with Jetty or at least have it clearly documented rather than have users guessing on whether they need to switch app servers or not. Part of this beta process is figuring out what exactly needs to go in those final release notes.

I'd be happy to help work with you on fixing Jetty, so please open a new issue with more details on what you ran into.

issuemover631 commented 7 years ago

Comment by hurricanehrndz Wednesday Mar 08, 2017 at 00:05 GMT


@muff1nman

Andrew, Thank you. I will open a new issue.