mqtt-bridge-smartthings icon indicating copy to clipboard operation
mqtt-bridge-smartthings copied to clipboard

Alternative way to set config directory

Open robross0606 opened this issue 4 years ago • 6 comments

Please provide an alternative means of setting the config directory, such as a CLI parameter. Having to set a global environment variable -- and then making that variable completely generic as "CONFIG_DIR" -- doesn't leave much room for flexible deployment. At the very least, it would be helpful if the environment variable was specific to this application (i.e. "MQTT_BRIDGE_ST_CONFIG_DIR").

robross0606 avatar Nov 27 '19 23:11 robross0606

Okay I officially hate iOS 13. Second time I tried to copy something from another app onto this and iOS logged me out of github and all the detailed response in lost.

Ok I hear the argument about if CONFIG_DIR is used globally that segregate this app from configs of other app, but i also hear about having central location for all all configs and logs so I don’t have to remember what is where.

If CONFIG_DIR is not specified it looks for config folder in root where mbs-server.js

Intentionally trying to avoid using CLI inputs as there are bash scripts, docker images etc where it will not play well uniformly.

Can’t specify in config file - chicken and egg - because I need this to find config file.

I had thought about maybe have a local env config variable but then confusing for users to understand where is config without looking at code. Maybe the logic I should implement is configdir = MBS_CONFIG_DIR || CONFIG_DIR || __dirName

It’s ugly because for most users who are running these in standalone docker containers or don’t use CONFIG_DIR because this is their only node application - redundant config variable is confusing and has the potential of pulling or storing stale information?

Thought? I can see the appeal but also the pitfalls.

At some level I agree with you - maybe the downside is limited in having another localized config Dir variable or just eliminate CONFIG_DIR completely and replace with MBS_CONFIG_DIR

sgupta999 avatar Nov 28 '19 12:11 sgupta999

If I were running on a unix platform, I would totally just do a local CLI environment variable to that line. However, doing that on the Windows platform is a total PITA. Using a regular command line argument unifies the approach so your app is consistent across OS platforms. If you're dead-set against allowing a CLI option like -c in addition to your other approaches, then your approach of allowing MBS_CONFIG_DIR as well as dotenv is probably the next best thing.

robross0606 avatar Nov 28 '19 13:11 robross0606

Okay for legacy people I will have to maintain CONFIG_DIR and MBS_CONFIG_DIR for now - but that will be a next version release.

I usually try to delay version releases till there is a critical mass of changes

The problem is the bin scripts - folks using those will have to modify those and that has potential for lots of error

Also even MBS_CONFIG_DIR will have to be a .env variable just as CONFIG_DIR that’s the only way I can access what path U want besides CLI

sgupta999 avatar Nov 28 '19 14:11 sgupta999

no i think somewhere I messed up with NPM - it picked up from my dev directory so startup is going to be messed up for everyone. The config directory is populated with my logs, config and state information.

Eeeesh . npm will not let me update without a version change.

the timestamps are also messed up 1985? - something weird happened here.

I will have to release v1.0.4 - sorry about the confusion.

sgupta999 avatar Nov 28 '19 15:11 sgupta999

just released version 1.0.4 on npm are you up and running?

sgupta999 avatar Nov 28 '19 16:11 sgupta999

think that might be meant for me in different threat! YES.... almost finished the iFan03 device handler. Just playing around with it now, might do two handlers one with light and one without so light can be on separate device.

rhamblen avatar Nov 28 '19 21:11 rhamblen