magfest / ubersystem

MAGFest's Ubersystem - handles ticketing, staffing, analytics, volunteers, and tons more
http://magfest.org
GNU Affero General Public License v3.0
48 stars 55 forks source link

As an administrator of ubersystem, I would like the option to have configuration stored in a database instead of in flat files #614

Open thaeli opened 9 years ago

thaeli commented 9 years ago

Placeholder for discussion of moving config from flat files to the DB.

binary1230 commented 9 years ago

so I guess there's 3 classes of configuration settings that I can think of: 1) big low-level config settings (like database user/password, Stripe account) 2) major config settings (like dates that registration opens, AT_THE_CON mode) 3) minor settings (like the logo images, etc)

So I don't like the idea of putting most of these into the database mostly because they're already living solely in the hiera file and that's a thing we can inherit from and copy around easily. It's source-controlled and we can do merging on it, and we can spin up new servers easily with it by just copying the config vs messing around with trying to do some kind of weird database merge.

I also don't like the idea that copying a database from one server to another might result in inadvertent config changes, or that you'd have to carefully think about which config changes you wanted to copy over from a backup, and which you didn't.

Also, even if we moved most of the config settings into the DB, we still have to maintain some config settings in external files, which means we need the existing system no matter what.

And, in a system where we have load balancing or replication, we'd need multiple identically configured servers. It's easy right now to push out the flat-file config changes as part of the deployment to multiple servers.

So, let me reframe this, are there specific configuration items you want to see exposed better to the end-user?

thaeli commented 9 years ago

I'm looking at this as moving towards a Twelve-Factor app. This has major advantages for deploying at scale.

This would break down the types of config setting differently:

  1. Config that varies between deploys (API keys, database connection string, hostname)
  2. Config that varies between events. (Everything else configurable)

I'd actually like to see just about everything exposed as config and stored in a backing service; I think the database is the most reasonable backing service to use.

On Mon, Oct 6, 2014 at 9:37 AM, Dominic Cerquetti notifications@github.com wrote:

so I guess there's 3 classes of configuration settings that I can think of: 1) big low-level config settings (like database user/password, Stripe account) 2) major config settings (like dates that registration opens, AT_THE_CON mode) 3) minor settings (like the logo images, etc)

So I don't like the idea of putting most of these into the database mostly because they're already living solely in the hiera file and that's a thing we can inherit from and copy around easily. It's source-controlled and we can do merging on it, and we can spin up new servers easily with it by just copying the config vs messing around with trying to do some kind of weird database merge.

I also don't like the idea that copying a database from one server to another might result in inadvertent config changes, or that you'd have to carefully think about which config changes you wanted to copy over from a backup, and which you didn't.

Also, even if we moved most of the config settings into the DB, we still have to maintain some config settings in external files, which means we need the existing system no matter what.

And, in a system where we have load balancing or replication, we'd need multiple identically configured servers. It's easy right now to push out the flat-file config changes as part of the deployment to multiple servers.

So, let me reframe this, are there specific configuration items you want to see exposed better to the end-user?

— Reply to this email directly or view it on GitHub https://github.com/magfest/ubersystem/issues/614#issuecomment-58017483.

EliAndrewC commented 9 years ago

I tend to share Dom's preference for having admin-exposed things in the database and doesn't-change things managed by puppet and in config files.

The main issue from my end is code complexity. Consider the age_group code, which ended up having to jump through a lot of hoops to be able to take a birthdate and then do a database query to get the id to set on the foreign key field. This ended up getting repeated in a bunch of places.

I don't have any real attachment to config files vs database config, but for me the real measure of which is better comes down to which ends up with nicer-looking code (which is admittedly a somewhat subjective judgement).

With that being said, there are some areas where it might make sense to move things into the database. For example, people commonly ask to add or remove columns to the schedule, or to add or remove departments. This currently requires us to fiddle around with config files in git, but really there's no reason why this couldn't be exposed through the user interface and then someone with access could just click a few buttons.

kitsuta commented 9 years ago

The ability to expose these options through the user interface is one the chief motivations behind moving these config files into the database. :)

Convention organizers are going to want a considerable amount of power when it comes to managing the registration system themselves. For example, in a different email thread, a convention organizer specifically mentioned the ability to turn off registration as a whole as a functionality that they wanted exposed through an interface.

Code complexity isn't great, for sure, but turning ubersystem into a usable software means making it work for a lot more use-cases than what MAGFest has. And believe me, there's a lot of different use-cases.

If we resort to tacking on if/else statements to handle each convention's specific needs, rather than designing a system that can accommodate most of them without alteration, we're still going to have code complexity. It'll just be in the form of useless spaghetti code. Given the choice, I'd much prefer to take a hit on complex database operations and session mixing than to spend the next five years stuffing the code full of statements like "if COLLECT_INTERESTS"

kitsuta commented 9 years ago

Though if we can refactor the birthdate code into something a little less bizarre, that would also be great.

Actually, a lot of these config options shouldn't be changed while registration is live, because doing so will trash existing data. I wonder if we can load in some of those options so that we can treat them a bit more like regular variables in code, rather than performing frequent calls to their table in the database?

EliAndrewC commented 9 years ago

Actually, a lot of these config options shouldn't be changed while registration is live

To me this is one of the dividing lines between database config and file config. When we're talking about config that is literally never changed once it's been set, then that sounds like config that belongs in a flatfile. On the other hand, when we're talking about config that is deliberately intended to change, over time such as what mode Uber runs as, I can totally see wanting to change that through the UI.

kitsuta commented 9 years ago

Yeah, and I think we could possibly do some things to those variables that we wouldn't do to variables that can change at any time (there must be a way to load stuff from the database into a dictionary of some sort, right?). But people are still going to want to change these options without a sysadmin or a programmer.

ksonney commented 9 years ago

OK, so here are my thoughts :

So if we launch an ubersystem instance with a pre-reg closing date of 12-31-14, but want to extend it by a week, I should be able to change that from the admin UI. I should be able to change "At the con" functionality from the admin UI. I should be able to add and remove departments, job locations, and pretty much anything from the admin UI except the server name, the database connection string, and security-related settings.

Conceptually speaking I should not have to restart the service to tell it that pre-reg gets extended a week, nor should I need to restart the service if I want to flip to "con is active and ongoing" mode (i.e. at_the_con).

binary1230 commented 9 years ago

revisiting this:

in light of the need to get this going probably soon for docker/etc, been thinking a lot about this and here's an approach I think could work out for everyone's needs. couple parts to it:

Part 1: Uber always reads from the config DB wherever possible. Exceptions are only for things that don't make sense (like credentials for the config DB).

Part 2: Populating the DB: Different events could do this differently and we could control it with event-specific or even server-specific config setting white/blacklists. Let's call it a blacklist that determines what the admin UI is allowed to let the admin users overwrite.

For cons not relying heavily on config management tools, the blacklist is empty, and the admins can change any config setting via the admin web UI.

For Magfest which would continue to rely heavily on config management tools, items on the blacklist are allowed to be set by puppet, and items not on the blacklist can be edited by the web UI. Any items on the blacklist would show up as grey on admin webUI ('ask your system administrator to edit this').

If an event didn't want to do any kind of manual configuration, the blacklist could contain everything (or, use an empty whitelist), and puppet would provide all config values.


The other good thing about this approach is if it makes sense to control more or less stuff in the web UI later, we can modify it just by changing the white/blacklist.

A puppet deploy would also no longer write INI files, instead it would write the config data directly to the redis DB. so regardless of where the config data is coming from (puppet or admin UI), it still only lives in the redis DB.

kitsuta commented 9 years ago

That sounds good to me!

I think maybe we should do it via a whitelist - probably safer to have new settings default to puppet-only than the other way around.

binary1230 commented 9 years ago

makes sense