Skip to content
This repository has been archived by the owner on Mar 7, 2018. It is now read-only.

Tiered configuration for embeddability on build #56

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

lloeki
Copy link
Owner

@lloeki lloeki commented Dec 9, 2015

  • configuration is embeddable
  • use OS conventions (via Electron) to store state

@lloeki
Copy link
Owner Author

lloeki commented Dec 9, 2015

Damn, forgot the doc. Will fix later.

@geekytime
Copy link
Collaborator

#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
Copy link
Owner Author

lloeki commented Dec 9, 2015

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:

  • %APPDATA% on Windows is critical and receives some important OS magic, e.g when installing as admin and running as a user. It typically resolves to C:\Users\YourUserName\AppData\Roaming\Matterfront.
  • $XDG_CONFIG_HOME or ~/.config on Linux, which is the now established way to not clutter $HOME with a thousand .dot entries.
  • ~/Library/Application Support is definitely the place where OS X apps are to put their various elements of configuration and persistent data. ~/Library/Caches is for cache and automatically handled as such by the OS. Temporary data ought to be placed in some /var/tmp or /var/folders/8n/00j6g_xx08qb21g22s_jj7sh0000gn/T/tmp.cP3XSC3U (mktemp -d output).

.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
Copy link
Contributor

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.

  • I do like the usage of appPath, as a way of distributing config info with the distribution. 👍 for those pieces
  • I think the getPath api seems stable based on a glance at related issues
  • I think we need to support the config methodology exposed by Add chrome config flags #54 ($HOME/.matterfront) 👍 for those features

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
Copy link
Collaborator

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

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

@LongLiveCHIEF
Copy link
Contributor

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.

var settingsDir = getSettingsDir(homedir);
return path.join(settingsDir, 'state.json');
var getConfigPath = function(appPath){
return path.join(appPath, 'config.json');
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll have to rename this to state.json, so as not to be confusing with the read-only config.json that lies in userDataPath.

@lloeki
Copy link
Owner Author

lloeki commented Dec 30, 2015

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
Copy link
Owner Author

lloeki commented Dec 30, 2015

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:

  • what's in matterfront (config.json, state.json) is used by dev-mode electron
  • what's in materfront/src is exclusively what gets embedded into app.asar (i.e src/foo.js is app.asar/foo.js, not app.asar/src/foo.js)
  • what's in userData is what gets read additionally, and the only RW path (i.e where state.json is stored).

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:

  • if $HOME/.matterfront exists, use that as the RW path instead of userData, but default to userData when it doesn't
  • make the RW path a runtime and/or build-time setting

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
Copy link
Contributor

@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
Copy link
Collaborator

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
Copy link
Contributor

You definitely explained that better than I did.

@lloeki lloeki force-pushed the tiered_config branch 2 times, most recently from 65a8e43 to 8395c40 Compare January 4, 2016 09:28
@lloeki lloeki changed the title tiered configuration for embeddability on build Tiered configuration for embeddability on build Jan 4, 2016
@lloeki
Copy link
Owner Author

lloeki commented Jan 6, 2016

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

@LongLiveCHIEF
Copy link
Contributor

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

💯

- configuration is embeddable
- use OS conventions (via Electron) to store state

be consistent: embedded config.json -> state.json
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants