Allow user to use env variables with gatewaystart.sh

[sblackstone]

A small change in gatewaystart.sh allows us to set environmental variables and retains the current behavior otherwise....

This patch still needs documentation etc; I figured I'd check in and see if this would be something useful to contribute before I do any further work on it.

[rlktradewright]

I've been mulling this over in my mind all day, and wasn't quite convinced, but couldn't put my finger on it. Nothing wrong with the concept, and undoubtedly there are use cases for it.

And then it hit me: there's actually no reason to do this. Since all the changes you suggested are in the part of the scripts that the user is 'allowed' to change, anyone who needs this can simply do it themselves. Indeed they could use any environmental variables they choose.

So I think it adds a bit of unnecessary complexity to the scripts that the vast majority of users will never use (but will nevertheless have to understand), and those who do want it can just do it anyway.

[sblackstone]

There are several use cases for this - this greatly simplifies Docker deployments without having to update .ini files for different environments and logins.

It's also more secure to just read the password from the env variable rather than having it stored in a file.

Also, less technically adept users aren't going to be editing this file anyways, they are going to just use the .ini file - there's really no reason or business for them to be looking inside shell scripts.

Finally, the implementation as it stands currently prevents me from using shell variables without specifically updating the distribution's files - something I'm loathe to do as I'd rather run with a stock distribution. If this file gets updated in a later version, I'll have to re-merge my edits.

What if instead of doing this, we just commented out the lines that set the variables to blank so if someone wants to put a value there, they just have to remove the # and set the value - it would be more obvious as to how to edit it.

Something like,

# If you want to set these, remove the # in front of the line
#
# TWPASSWORD=
# TWUSERID=

---- (warning about don't edit below this line) ----

TWSUSERID=${TWSUSERID:-""}
TWSPASSWORD=	TWSPASSWORD=${TWSPASSWORD:-""}

This would still make it more obvious as to how to edit it, but leave in place the ability for users to just use shell variables as they are intended.

[sblackstone]

I changed my patch to reflect what I said above, this might be a good comporomise between ease and features.

[rlktradewright]

1. Supplying the password from the script is not secure: it goes in through the IBC command line args, and is then visible to anyone via ps. Regarding encrypting the config.ini file you might find this thread in the IBC User Group interesting.

2. Every user is going to have to edit the top-level scripts at some point, for example to set the TWS/Gateway version number. Having naïve users edit scripts or configuration files is always somewhat unsatisfactory, and that's why I've gone to great efforts to make the process as simple and foolproof as possible, and in general it seems to work well - people seem to be able to cope with this.

3. Your revised patch just makes it even more obscure. Now the user has to work harder to see what the default is.

Given that you're going to have to edit this file at some point (see 1.), you might as well just apply your proposed enhancement to your copy. You only have to do it once, you only need to do it for the variables that you actually want to pass in (and you may not need the defaulting), and you can give your external environmental variables whatever names you want.

So I still don't see any real value in this proposal: the downsides for the many outweigh the very minor convenience to you.

[sblackstone]

The code seems to use env vars to pass from gatewaystart.sh -> displaybannerandlaunch.sh, so clearly they are a useful way to pass information between programs.

Everything would work fine if gatewaystart.sh didn't purposefully overwrite the variables I'm passing in with its own defaults. Environmental variables have been a standard way to pass data to an application since the 70s and it seems like we're going out of our way to ignore that useful convention.

Why the sudden switch in ibcstart.sh to using command line params? It seems like if the underlying Java code just read the environmental variables as well you could avoid the whole problem of it showing up in ps..

[rlktradewright]

Environmental variables are fine for things that are environmental - the clue is in the name! That means that they keep the same value from invocation to invocation (or at least change relatively infrequently).

But a username and password are absolutely not environmental. I run two instances of TWS in the same (computer) user account, live and paper, and with your scheme this would be more difficult.

Note also that if you're running more than one instance of IBC, they have to have different logfile directories, so again, not really environmental.

Also, there are many ways to get information into a program. I suspect args to the main() precede even environmental variables. So please don't try to claim that 'we're going out of our way to ignore that useful convention'.

So, bottom line: this isn't going to fly!

[sblackstone]

Things like passwords and usernames being stored in environmental variables are widely used, for example applications will store database connection information in a variable like MYSQL_PASSWORD or MYSQL_HOST.

By doing this, if you run the application on say a developers laptop, it will connect to one database while if you push it to production it will pick up the database thats running there without having to keep track of two separate configuration files...

So your bottom line is the difficulty of making users remove a # in the front of a configuration line outweighs the benefits of adding flexibility for more sophisticated users?

[rlktradewright]

I've just spent half an hour typing a reply, and the damn Github webpage went blank, refreshed itself, and wiped it all out!...

I have other things to do now, so I'll get back to you later.

[sblackstone]

Alas! Please don't take any of this personally by the way - I do appreciate that IBC gets the job done and saves us alot of time. If we were to sit down for a beer, we could have great times recounting our tales fighting with the IB API to get very basic things done.

[rlktradewright]

Don't worry, I'm not taking it personally!

Sitting down for a beer, or even standing up for one (maybe just the first one!), sounds like a good idea. So next time you're passing through rural Aberdeenshire...

Ok, rather than debate the merits of various means of getting data into a program, let's go right back to square one. What I need to understand is what your real requirement is, and I believe that what's not satisfactory for you is that at present, you can't just download the IBC zip, unpack it, and run it without further amendment? Please tell me if that's not what you're concerned about.

Now, I perfectly understand why you might want to be able to do this within your Docker image, but there are two points I want to make about this.

First, what is it that you should download? I'm not sure that downloading the zip file direct from Github for this purpose is 'nice', since it's effectively treating Github as a content distribution network, and they might take a dim view of that. Of course IBC is absolutely tiny, so they'd probably never notice, but there's a principle here and it might be worth checking with them whether they have a policy about this.

Secondly what I'd like to suggest is that maybe what you should be downloading is something different than the standard zip distribution file, ie it could be something that's more tailored for Docker-type purposes. I'll explain why below.

Now, regarding the scripts, I can actually see a compromise solution that gives you directly what you want in terms of using environment variables (I'm tired of calling them 'environmental'!). This is very simply to move your modified version to the scripts subfolder (and perhaps remove all the comment verbiage), and to call it from the higher level scripts. This means that the top levels scripts are themselves merely setting the variables expected by the lower level script (and there only needs to be one of these since the APP variable is already set at the higher level).

In this way, there's no need to change the top level scripts at all, which I really do want to avoid if at all possible. But in your Docker, you can simply set the TWSPASSWORD and TWSUSERID variables as required and call the lower level script directly.

I like this suggestion and have tried it out here and it works fine (well of course it does!).

So that gets you most of the way to what you want: the other thing is the ReadOnlyApi setting in config.in. I'm not going to change the default setting for this to "no", because that forces IBC to open the configuration dialog and set the checkbox every time you run it, even though TWS/Gateway remember the setting correctly across sessions. Now, as I alluded to above, one way to approach this would be to have a different download that is customised for Docker use, with a config.ini that has ReadOnlyApi=no, and perhaps doesn't include the stuff that you wouldn't need in Docker such as the User Guide. But it's probably easiest to simply put your own config.ini into your Docker image and set the CONFIG environment variable appropriately.

Just for completeness, it's probably also worth pointing out that there's nothing to stop you calling the displaybannerandlaunch.sh or ibcstart.sh scripts directly, using the values from your own environment variables, and ignoring the top-level scripts altogether.

So even without the script refactoring (which as I say, I do like), there are options for the more sophisticated user.

But if you'd like to modify your PR along the lines I've suggested, I will look on it much more favourably.