Make runservers a little smarter

[kentquirk]

It now works better (doesn't leave you in random directories) if your setup isn't correct. This also moves the ugly styx rules out into a separate JSON file.

[bewest]

Looks good to me. :+1:

[cheddar]

@kentquirk Why remove the styx config to a separate file. I'm gonna be an ass here, but quoting from the email conversation I started when I didn't like the ugly configs:

"I'm not sure I get the difference between editing a JSON file and editing an environment script."

[bewest]

Count the backslashes?

To quote myself, environment files are good for key=value pairs where there are around 7, items. For complicated things like styx you are better off using one of those key=value pairs to name a config file.

Let's try to keep things as simple as possible. I'll follow up with a patch to styx to actually do that if you like.

Not all daemons need a complicated config, if one does then fine, but let's not be editing files that are so full of backslashes it's hard to maintain.

On Fri, Feb 14, 2014 at 10:12 PM, cheddar notifications@github.com wrote:

@kentquirk https://github.com/kentquirk Why remove the styx config to a separate file. I'm gonna be an ass here, but quoting from the email conversation I started when I didn't like the ugly configs:

"I'm not sure I get the difference between editing a JSON file and editing an environment script."

Reply to this email directly or view it on GitHubhttps://github.com/tidepool-org/tools/pull/2#issuecomment-35148568 .

[cheddar]

@bewest, yes, I get why it's ugly. That's why I created an email thread. That same email thread discusses why I didn't want to have an environment variable that points to another file (it creates extra assets that must be managed in order to run the thing).

Fwiw, I'd personally prefer to switch the styx config entirely over to a JSON file, but we are using environment variables.

[kentquirk]

I was thinking of the runservers stuff as a developer-only setup, and I didn't think of it as a particularly challenging thing to manage the separate file -- and I would much rather do that than try to deal with all the escaped quotes and backslashes.

The reason I pushed us to the environment variables was that I thought we were going to be setting them in the Amazon control panel. But with Shio, the configurations are just pulled from a repository -- so I'd be perfectly happy if all configuration became a JSON file and we eliminated the env.js thing. Sorry I didn't say that before.

[bewest]

We're doing what simple and wise to do, not sticking to a policy because that's a policy.

There is no strict policy on "we only use enviroment" variables like you are suggesting.

Most things are simpler enough to only need a few settings. Styx requires a big chunk of rules. A config file is perfect.

-bewest

On Sat, Feb 15, 2014 at 10:21 AM, Kent Quirk notifications@github.comwrote:

I was thinking of the runservers stuff as a developer-only setup, and I didn't think of it as a particularly challenging thing to manage the separate file -- and I would much rather do that than try to deal with all the escaped quotes and backslashes.

The reason I pushed us to the environment variables was that I thought we were going to be setting them in the Amazon control panel. But with Shio, the configurations are just pulled from a repository -- so I'd be perfectly happy if all configuration became a JSON file and we eliminated the env.js thing. Sorry I didn't say that before.

Reply to this email directly or view it on GitHubhttps://github.com/tidepool-org/tools/pull/2#issuecomment-35163323 .

[kentquirk]

Actually, we DID decide to create a policy on this, and in my zeal to eliminate the hard-to-read embedded configuration, I forgot. We had an email thread called "Config Library" where we discussed this as a team and settled on a plan. That plan was, to quote the thread:

"We define all config in an env.js file, we never ever ever look at process.env anywhere else, the environment variables can be set on deploy and that one file is our documentation."

So my patch was actually wrong and I'm going to revert it.

If we want to have another conversation about this, we can have it after the pilot. I can put it on the ever-growing list of Things We Need To Discuss.

[bewest]

Ok, I don't plan on following this policy. That doesn't make sense to me, sorry.

I don't see how using env.js to read the name of a config file from the environment is a failure to follow the policy although clearly some people feel it is.

-bewest

On Mon, Feb 17, 2014 at 8:19 AM, Kent Quirk notifications@github.comwrote:

Actually, we DID decide to create a policy on this, and in my zeal to eliminate the hard-to-read embedded configuration, I forgot. We had an email thread called "Config Library" where we discussed this as a team and settled on a plan. That plan was, to quote the thread:

"We define all config in an env.js file, we never ever ever look at process.env anywhere else, the environment variables can be set on deploy and that one file is our documentation."

So my patch was actually wrong and I'm going to revert it.

If we want to have another conversation about this, we can have it after the pilot. I can put it on the ever-growing list of Things We Need To Discuss.

Reply to this email directly or view it on GitHubhttps://github.com/tidepool-org/tools/pull/2#issuecomment-35298606 .

[kentquirk]

We agreed as a team that configuration belongs in the environment. So we're going to keep it there until we decide as a team to move it elsewhere.

[bewest]

Right, I think we have a disagreement on what "in the environment" means.

RULES=$(cat styx_rules.json)
$ RULES="$RULES" node server.js

Both above and below example define everything via the environment variables. For some reason people are claiming it's not. This confuses me. I prefer the one below, but the above is acceptable for now.

$ RULES="./path/to/styx_rules.json" node server.js

The following, for contrast, would break the policy, and so I won't do it:

$ node server.js --rules ./path/to/rules.json

However for this use case, I would vastly prefer one of the bottom two examples. Only the last example violates the policy, correct? The reason being logic given through command line switch instead of environment variable.

-bewest

On Mon, Feb 17, 2014 at 9:14 AM, Kent Quirk notifications@github.comwrote:

We agreed as a team that configuration belongs in the environment. So we're going to keep it there until we decide as a team to move it elsewhere.

Reply to this email directly or view it on GitHubhttps://github.com/tidepool-org/tools/pull/2#issuecomment-35303810 .

[bewest]

I would appreciate getting clarification on this. The interpretation of the rule from some people seems different from what I would naturally expect.

ok, uses environment variable to provide contents

RULES=$(cat styx_rules.json)
$ RULES="$RULES" node server.js

ok, uses environment variable to specify config

$ RULES="./path/to/styx_rules.json" node server.js

not ok, uses command switch to specify config

$ node server.js --rules ./path/to/rules.json

This is what I would naturally expect given the way the rule was communicated to me. Is this different from what other people expect?

[cheddar]

I think this is a better conversation to have once we have actually shipped UCSF and have breathing room.