passiomatic / coldsweat

Web RSS aggregator and reader compatible with the Fever API
MIT License
146 stars 21 forks source link

Add setup configuration so coldsweat can be installed as a Python package #96

Closed SSheldon closed 8 years ago

SSheldon commented 8 years ago

With this change, all you have to do to run coldsweat is pip install the package, add a config file, and then you're set. No need to fork and clone this git repo :) Then, when it's time to update, it's just a matter of updating the package rather than having to merge upstream changes back into your branch.

I think this can make coldsweat easier to use and maintain, so I'm excited about it, but it is a fairly different strategy so there might be downsides to this I haven't considered. Let me know what concerns you have.

Testing done:

SSheldon commented 8 years ago

Oh, and if you do like this I'm happy to update some documentation to explain how to setup coldsweat from a package :) I've been running it like that for a few weeks now and its worked great.

passiomatic commented 8 years ago

This is great, thank you. One thing that I don't understand is where do you install the package if you need to use the web UI? One portion of Coldsweat (/static and CGI, FastCGI ecc. ) needs to be visible from a web server.

SSheldon commented 8 years ago

Running CGI/WSGI from the package is pretty easy, you just basically need to copy the wsgi.py/index.cgi that are currently in the repo. For example, in my deployment from a package I had to write my own wsgi.py file, but that's no biggie since it's just two lines: from coldsweat.app import setup_app; app = setup_app().

The static files, I'm not sure the best way to handle. If you sweat serve right now, it'll work because the static files are included in the package. If you want to serve the static files elsewhere, like a CDN, you need to copy the files over to there. Having users copy the files out of the package by hand isn't ideal, maybe we'll want to add a... sweat install command or something that will copy the static files into a directory so that your webserver can serve them itself.

passiomatic commented 8 years ago

Makes sense. I'm gonna merge the change into the 0.9.6 branch and I'll have a look at the whole approach at later time. It is definitely an interesting thing to have Coldsweat configured as package.

Deploy on a web server isn't easy anyway since one has to know a few details already. If we simplify at least the download phase it will be good. We we will need to write some docs. For a start can you please update the Setup-0.9.6 wiki page with the new instructions?

I suppose the pip install coldsweat command will download the dependencies too?

SSheldon commented 8 years ago

Awesome! Yep @passiomatic, pip install coldsweat will also install the dependencies of Coldsweat.

I've updated the Setup-0.9.6 wiki page, let me know if there are any parts that should be clarified :)

passiomatic commented 8 years ago

Great. Another thing, we need to specify the exact versions for the Coldsweat dependencies, since sooner or later something will break. Peewee in particular changes at a fast pace and API changes are the norm.

tewe commented 8 years ago

This makes paths rely on the current working directory instead of where the module lives, which is a problem when importing it from elsewhere, e.g. when unit testing plugins.

passiomatic commented 8 years ago

Another thing, we need to specify the exact versions for the Coldsweat dependencies, since sooner or later something will break. Peewee in particular changes at a fast pace and API changes are the norm.

Done in 8278f217d5abf5c869827bd78aab3bcf06c283bb.

passiomatic commented 8 years ago

This makes paths rely on the current working directory instead of where the module lives, which is a problem when importing it from elsewhere, e.g. when unit testing plugins.

@tewe Do you think it is fixable by introducing a new plugins_dir config option? One thing that bugs me with this package approach is that we have a way to specify the log file path, the sqlite path but not the where the plugins directory lives.

tewe commented 8 years ago

Plugins aren't the problem. If you import coldsweat it reads etc/config in the current working directory. If you're not in the root directory of a coldsweat installation, this will fail. One reason you might not be is that you're developing a plugin, or any other code that tries to use parts of coldsweat. This is not how you write a module.

passiomatic commented 8 years ago

I understand your concern. I'm trying to do two different things here: keep Coldsweat a standalone package to make it easier to keep it up-to-date and run it as an webapp.

If you import coldsweat it reads etc/config in the current working directory. If you're not in the root directory of a coldsweat installation, this will fail.

Yes, this is an problem that needs to be addressed before the 0.9.6 release. That's why I reopened #92.

Thinking more broadly: I'm wondering if decoupling the automatic loading of a config file currently in init would make Coldsweat more usable as a standalone module. What if the loading of the config information would be started from the sweat command line utility or the various scripts devoted to the deploy of the web app? Food for thought.

tewe commented 8 years ago

(If an app as opposed to a library comes as a package I put it in a virtualenv anyway, so I actually prefer to just use git to keep those up-to-date)

Shared code that requires configuration is why we have classes. Global variables like template_dir or logger are not a good idea for a module.

But those variables can be changed after import, so the only problem is reading the configuration file. If you're hinting at moving that into a function, I heartily agree. As using coldsweat as a module is currently hard, I doubt many people do it, so you can afford that change.

passiomatic commented 8 years ago

Quick update: I've moved all the changes made by @SSheldon in a separate issue92 branch since I needed to release version 0.9.6 which contains several important fixes to the fetcher.

Changes proposed by @SSheldon are interesting but to work convincingly in any scenario they require some additional work (you can read some work need to be done in #92).