lloeki / matterfront

Mattermost frontend app for OS X, Windows and Linux
MIT License
152 stars 27 forks source link

Tiered configuration for embeddability on build #56

Open lloeki opened 8 years ago

lloeki commented 8 years ago
lloeki commented 8 years ago

Damn, forgot the doc. Will fix later.

geekytime commented 8 years ago

54 adds the same config file, so we're definitely on the same page, there.

I'm not sure about using Electron's appPath and userDataPath, though. I'm really used to editable config files being stored in the user's home directory for most apps, across most platforms. This is especially true of Node apps.

It's been a long time since I saw an app that stored any kind of data in the app directory. That never seemed like a good idea to me, and could definitely cause problems for users that are not administrators on their machine.

I'm also not sure about that userData path. Those paths have always felt to me more like places where apps can dump temporary data like browser cache, downloaded zip files, etc. Human-readable configuration data should be easier to get to, and in a place that is more familiar to humans.

lloeki commented 8 years ago

appPath refers to asar content (or the first argumetn to the electron command when run in dev), which is what makes the config embeddable. That is, on making an internal build a company can include a default config whose values will be defaulted to upon startup, without any user intervention. IIUC This is different than what's in #54, which is apparently about having a non-write config in the user's config directory.

getPath(userData) points to the platform-specific directory where config ought to be stored:

.dotfiles on both OS X exist mostly in a Unix CLI realm, and as far as I remember Windows isn't even allowing leading dots in filenames from the UI (although NTFS and exfat filesystems do support it)

Using home dotfiles anywhere else than Linux (and other similar Unix) is pushing a non-platform-specific habit on a foreign OS. Even on Linux, the recommended way is to use XDG_CONFIG_HOME, even if many a piece of software ignore this because history.

LongLiveCHIEF commented 8 years ago

If think something of possible significance here is that we're not "installing" the application, we're just "running an executable". IIUC these directories are heavily relied upon for applications that rely on swap/cache/heap, because they internalize state, and register services and utilize OS lifecycle events/policies. For our app, most of the state actually exists on the server, and we're not really utilizing too many native api's (yet?)

I actually think both methods provide value, because we have 2 different contexts in which the app is run, (launched via node, and launched via executable). For now I don't think we need getPath, and I think the deciding vote on that one should be how stable the getPath api is, and whether it's more hassle than it's worth/if there's anything to gain from using it.

If we do implement getPath, I think it makes sense to allow nconf to use $HOME/.matterfront configs from #54 to override the configs loaded from the getPath api, as these would most likely be used for individuals acting as collaborators, those doing relevant plugin development, or those who need custom setup.

We need to make sure that when an admin runs a build, the admins own user configs don't leak into the embedded configs distributed with a build. Perhaps we should use both methods, and structure one to support builds and dev-mode runtime, and the other to support runtime for distributed builds.

note: when running.... matterfront launched from node the process will show in the OS as electron, whereas in the builds, it will show as matterfront. perhaps we use getPath when the process is matterfront, and $HOME/.matterfront when the process is electron?

geekytime commented 8 years ago

I do like the usage of appPath, as a way of distributing config info with the distribution. :+1: for those pieces

Config that ships with the distribution should definitely be included as defaults in nconf. Never as another layer of config.

LongLiveCHIEF commented 8 years ago

Config that ships with the distribution should definitely be included as defaults in nconf. Never as another layer of config.

I'm not talking about our distributions, I'm talking about organization specific distributions.

lloeki commented 8 years ago

Indeed, this is only of use to org builds, preseeding state with org-specific values so that it works out of the box within an org. After first start, users can do whatever they wish and should they screw things up, fix everything out just by removing their userDataPath dir.

lloeki commented 8 years ago

We need to make sure that when an admin runs a build, the admins own user configs don't leak into the embedded configs distributed with a build

Initially I implemented that stuff this way:

perhaps we use getPath when the process is matterfront, and $HOME/.matterfront when the process is electron

I'm very open to unconditionally adding $HOME/.matterfront as an additional lookup path for config.json.

Also I'm open to any or all of:

But really, what I want to know is the precise use case of @geekytime. IIUC it to separate the "electron layer" from the "matterfront layer", whereas what @H3Chief is mentioning is having two sets of config, one for contrib/admin and one for a built release/deployed user (which may happenn to be on the same machine, hence the need for a distinction for them not to behave the same).

I won't go forward one way or another until the use cases are made explicit and sorted out.

LongLiveCHIEF commented 8 years ago

@geekytime had a really long and opinionated chat about this, and in the end, I came to see his point of view regarding config layers. When summed up, I'd say @geekytime's pitch is that configuration be super small and minimal, so that one product can be released from our downloads/tags for those who want a build, (but still have the option to build themselves if they want), but for that configuration to be repeatable as small units for both large orgs and small teams alike.

With the code in #67, what you wind up with is a team switcher just like the one you have with Slack's native client, which will support both multiple teams and multiple servers, in addition to multiple organizations.

The org-level stuff that I was concerned about became pretty simple to reason about once the idea wound up being reduced to smaller units instead of more robust packaging/distribution. Each time an end user signs into a new team, a new team-button is added to their switcher, regardless of the organization, server, or team affiliations they have.

@geekytime did a much better job of expressing it than i did, but we had the benefit of a VOIP convo for quite a while. If you can meetup with us on Google hangouts, I think you might have the same light bulb moment I did.

67 uses exclusively $HOME/.matterfront for config lookups (in addition to any cli params passed in during startup), but for now, I'll wait to discuss it until we can either chat in person, or let @geekytime chime in on it.

geekytime commented 8 years ago

So the gist of what I told @H3Chief goes like this. Most software doesn't have to be rebuilt to be configurable by admins. I understand the use cases of wanting to be able to distribute a customized config with the app, but that isn't something that Matterfont itself should have to know about. Matterfront simply needs to know how to read a per-user config file, and save and re-read a per-user state file.

When enterprise admins need to distribute customized config, they should distribute customized config not a customized build of the whole app. There are lots of ways to do this:

  1. Most enterprise issuance systems are scriptable. Simply deploy default custom config/state files along with the standard build of Matterfront.
  2. Create a separate Matterfront installer package that accepts custom config via a UI, or as an input file. Then admins could execute the installer and pass it their default config.
  3. Add a separate Matterfront configurator that reads a config.json from a remote location, such as the Mattermost server. (This option would also allow people to be pre-configured into teams via LDAP/etc.)

There are probably even more ways to do this, but those are the ones I can think of right now. Forcing admins to build their own Matterfront is not only cumbersome, but also adds complexity to the Matterfront project, and also shoe-horns our users into choosing basically one of the above options. If we keep Matterfront simple, it will allow our end-users and their admins to choose the configuration option that works best for them. We can support whichever of the options above that we need, but they can still roll their own from scratch without any of us ever having to touch Matterfront code to do it.

LongLiveCHIEF commented 8 years ago

You definitely explained that better than I did.

lloeki commented 8 years ago

Good job explaining your PoV. Just a note that I'm not letting this linger, I'm effectively pondering a properly articulated answer.

LongLiveCHIEF commented 8 years ago

Just a note that I'm not letting this linger, I'm effectively pondering a properly articulated answer.

:100: