engine-deprecated icon indicating copy to clipboard operation
engine-deprecated copied to clipboard

Any command accept `--config` argument but it does nothing

Open smacker opened this issue 6 years ago • 6 comments

$ ./srcd --config custom_config.yml sql "select * from repositories"
+---------------+
| REPOSITORY ID |
+---------------+
| engine        |
| aws-sdk-php   |
+---------------+

Only init should accept it then.

smacker avatar Mar 04 '19 17:03 smacker

The problem is that the user is not required to run init before sql. So if the daemon container is not running, that srcd sql --config command will start the daemon with the given config.

Maybe this can be explained more clearly in the docs or the CLI help.

carlosms avatar Mar 05 '19 12:03 carlosms

Makes sense. Though it's inconsistent then:

  1. there is no daemon and I pass --config to srcd sql => it applies config
  2. there is a daemon already running and I pass --config to srcd sql => config is ignored

Require user to know if the daemon is running... It's not the best idea in my opinion. Daemon can die by itself, or when user restarts docker and reboots system (which is very common on windows)

smacker avatar Mar 05 '19 12:03 smacker

Require user to know if the daemon is running...

I agree that this isn't the ideal thing. IMO there are two possibilities:

  1. enforce running srcd init first (will fix also #254),
  2. always honour config independently from the daemon running or not, so that the behaviour is consitent.

The problem with the second solution is that if the daemon is running and notices a different configuration (for example a port change), should then decide and eventually handle the restart of the components whose conf has changed.

se7entyse7en avatar Mar 05 '19 18:03 se7entyse7en

I also found this issue during my https://github.com/src-d/empathy-sessions/issues/37#issuecomment-470037191

If you log with --verbose it is even said that the config will be used but it won't be the truth:

$ srcd init /path/to/scan --config conf/conf.default.yml --verbose
DEBU[0000] Using config file: /home/david/.srcd/conf.default.yml

$ srcd sql "SHOW tables;" --config conf/conf.yml --verbose
DEBU[0000] Using config file: /home/david/.srcd/conf.yml
FATA[0000] could not start gitbase: could not start container: listen tcp 0.0.0.0:3306:
bind: address already in use

imo, if it is passed a different conf, compared with the one used to initialize the daemon, it should raise an error, and maybe a hint recommending to init again, for example:

FATA[0000] Passed config differs with the used to initialize the daemon.
Run `srcd init` again with the new config in order to make it active.

srcd init /path/to/scan --config ~/.srcd/conf.yml --verbose

The alternative of restarting engine can be too much because there can be queries running on background, or even other external clients using already created containers.

Enforcing the users to run init again on demmand, let's them know that everything will be restarted.

dpordomingo avatar Mar 06 '19 09:03 dpordomingo

Now all commands respect config passed to init and srcd sql --config would be ignored even if the demon is stopped. The only case when it would work now: after the very first run of engine or after prune.

smacker avatar Apr 02 '19 11:04 smacker

imo to ignore the passed config is a misleading behavior; if the stored config differs from the passed one, srcd should say so: failing, restarting with the new config, or warning the user.

dpordomingo avatar Apr 03 '19 03:04 dpordomingo