mattock / automatic-cloud-backup

19 stars 19 forks source link

Fixes #6 add parsing for ENVVARS #7

Closed bgupta closed 7 years ago

bgupta commented 7 years ago

@dzschille @mattock Cancelling all PRs and issues.. I'll run my own fork and will no long participate in this upstream.

mattock commented 7 years ago

@bgupta well, we can't stop you from doing that. Personally I think that is an overreaction to one rejected PR.

This PR does have potential. So goal is to allow defining or overriding the variables (e.g. INSTANCE) on the command-line, right? If I read this right, if any of the variables is unset, the script will source the the config file. The problem there is variables in the config file will then have precedence, which is probably not intended. But I'm just guessing as you did not provide a description of why this feature would be good to have.

In any case, one way to allow overriding individual variables on the command-line is this construct:

INSTANCE="${INSTANCE:-default_instance}" 

This would be put to the configuration file (including the sample).

bgupta commented 7 years ago

Logic is that in the ideal, we shouldn't require a config file, and should be able to send all config as either flags or ENV_VARS. Thinking the following order of precedence if the values are being set in multiple places.

1) flags/args 2) ENVVARS 3) configfile

Eventually my goal is to allow individual settings to be set from any of the three, but that is a much bigger refactor. So, for now I am assuming that if you want to use ENVVARS you are doing so en masse, and don't want to manage a config file, as is my case. ENVVARS was easier to implement than flags.

I plan to make a lot of changes, and realize now that it will be painful/time-consuming to try and convince others to accept my changes. I've already got the code with umask running in prod, I believe it is the cleaner approach, and as such don't want to go backing out my changes because folks don't agree with me. It's a small enough project/codebase that trying to keep in sync with upstream probably isn't worth the effort, if it's going to be a hard sell for every change.

mattock commented 7 years ago

@bgupta : yes, if you need to customize your version heavily then having a private fork is probably easiest.

That said, I agree that the flags/args -> ENV vars -> config file approach you suggest makes sense. What other changes do you plan to make?