make database config optional

[ishidawataru]

Thie PR made the existence of the database config mandatory.

To preserve previous behavior, I think it's better to make the config optional.
Is it possible to change the implementation to use DEFAULT_UNIXSOCKET if the database config doesn't exist?

[dzhangalibaba]

we don't want to use DEFAULT_UNIXSOCKET anymore, later we will always have database config file which is built in images by default. Everything related to DB will be read from the config file in future. database config doesn't exist is a invalid case in furture.

[ishidawataru]

Thanks for your prompt response.
In my use case, I'm not using the official SONiC container image. ( I admit it is a rare use case )
It would be nice if this library can be used easily with fewer dependencies.
I think it will be beneficial for testing and debugging as well.

[qiluo-msft]

@ishidawataru Could you give a use case? The new feature should be backward-compatible by design.

[ishidawataru]

@qiluo-msft Actually, I was suggesting to maintain the backward compatibility which was broken by #319.

Before the PR, I could use this library without database_config.json. But the PR made the existence of the file mandatory.

[qiluo-msft]

@ishidawataru Yes, we are considering this. I am asking for a true use case so we could improve the implementation for backward compatibility. Do you have some sample code?

[ishidawataru]

I'm not using the official SONiC container image. I'm developing a minimalistic SONiC package that could be installed on any Linux distribution.
I'm not sure I can say this is a true use case though.

PoC code here. https://github.com/ishidawataru/usonic

[qiluo-msft]

Offline discussed with @ishidawataru, this issue is a feature requirement. Use case:

1. If database_config.json is provided, use it
2. If no database_config.json at all, fallback to single Redis instance.

Currently swss-common library's downstream modules are using dbName string to access a Redis database, we need to implement a mapping from dbName to dbID (integer) without a configuration file.

[dzhangalibaba]

Offline discussed with @ishidawataru, this issue is a feature requirement. Use case:

1. If database_config.json is provided, use it
2. If no database_config.json at all, fallback to single Redis instance.

Currently swss-common library's downstream modules are using dbName string to access a Redis database, we need to implement a mapping from dbName to dbID (integer) without a configuration file.

Today database_config.json will be placed into the correct location when swsscommom deb pkg is installed. The default context is the same single dB mode as earlier(port, socket are the same as before). All new Lib API will read this file automatically and old API still works since it is not removed from swsscomm lib.

[dzhangalibaba]

I believe the config file installation is not there when the issue is filed. Later we add this feature.

[ishidawataru]

If the library can't make the configuration file optional, can it have an option to change the location of the configuration file on runtime by using an env variable?

Also, it'd be nice if we can control which method, network or unix domain socket to use to access the Redis in this configuration file. ( maybe I should create a new issue for this? )

[qiluo-msft]

@ishidawataru The environment support is not available. But if you want to runtime change path, you could call SonicDBConfig::initialize(PATH) at the very beginning of your application.

You mean TCP/unixsocket by "method"? It is currently part of code, and the config file has both. The TCP port and unixsocket path could be configured in this file.

Let me know if current implementation could cover all your requirement.

[ishidawataru]

@qiluo-msft Is it acceptable if I submit a PR to make this configurable by using an env variable?
I'd like to use SWSS applications as is if possible.

You mean TCP/unixsocket by "method"?

Yes. In my understanding, currently, the application is choosing which to use TCP or Unix socket to access the database. Can we force the application to use one method ( for example TCP only ) by using the configuration file? By supporting this, we can have remote Redis instance which can't be accessed via Unix socket.

[qiluo-msft]

Offline discussed with @dzhangalibaba @ishidawataru

1. add environment variable to swss-common/swsssdk-py/telemetry to specify database_config.json path
2. add new field in database_config.json to override TCP/unix_socket choice which previously specified in code. Could be override to TCP, or to unix_socket

We are agree on Item 1. @ishidawataru will work on it.
We are not fully agree on Item 2. Need more disucssion.