Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[MultiDB]: throwing exception when database_config.json doesn't exist #319

Merged
merged 6 commits into from
Dec 4, 2019

Conversation

dzhangalibaba
Copy link
Contributor

  • throwing exception if config file is missing
  • using interface .at() to get map entry

Signed-off-by: Dong Zhang d.zhang@alibaba-inc.com

@dzhangalibaba
Copy link
Contributor Author

@kcudnik @qiluo The exception part.

@qiluo-msft BTW , we need to make a agreement on whether adding db config file into build docker by default to make progress on next step. Kamil and I thought it is OK.

@@ -53,42 +53,44 @@ void SonicDBConfig::initialize(const string &file)
{
throw runtime_error("Sonic database config file syntax error >> " + string(e.what()));
}
} else {
Copy link
Contributor

@qiluo-msft qiluo-msft Dec 3, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

} else { [](start = 4, length = 8)

follow code format nearby #Closed

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DONE

@@ -53,42 +53,44 @@ void SonicDBConfig::initialize(const string &file)
{
throw runtime_error("Sonic database config file syntax error >> " + string(e.what()));
}
} else {
throw runtime_error("Sonic database config file doesn't exist at " + file);
}
}

string SonicDBConfig::getDbInst(const string &dbName)
Copy link
Contributor

@qiluo-msft qiluo-msft Dec 3, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

getDbInst [](start = 22, length = 9)

Add a unit test case for non existing dbName #Closed

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DONE. Added all five API test cases.

}
}

string SonicDBConfig::getDbInst(const string &dbName)
{
if (!m_init)
initialize();
return m_db_info[dbName].first;
return m_db_info.at(dbName).first;
}

int SonicDBConfig::getDbId(const string &dbName)
Copy link
Contributor

@qiluo-msft qiluo-msft Dec 3, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

getDbId [](start = 19, length = 7)

Add a unit test case for non existing dbName #Closed

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DONE. Added all five API test cases.

Copy link
Contributor

@qiluo-msft qiluo-msft left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As comments

@kcudnik
Copy link
Contributor

kcudnik commented Dec 3, 2019

for each throw can you also add swss_log_error message to make mark in syslog if on someplace someone will forget to catch/log catched exception?

@dzhangalibaba
Copy link
Contributor Author

Capture Logs When exception Thrown

Dec  4 08:08:55.621594 ASW-7005 DEBUG syncd#syncd: :> syncd_main: enter
Dec  4 08:08:55.621837 ASW-7005 ERR syncd#syncd: :- initialize: Sonic database config file doesn't exist at /var/run/redis/sonic-db/database_config.json
Dec  4 08:08:55.623204 ASW-7005 INFO syncd#supervisord: syncd terminate called after throwing an instance of 'std::runtime_error'#015
Dec  4 08:08:55.623289 ASW-7005 INFO syncd#supervisord: syncd   what():  Sonic database config file doesn't exist at /var/run/redis/sonic-db/database_config.json#015

UT tests:

[ RUN      ] DBConnector.multi_db_test
Default : isInit = 0
INIT: loading nonexist db config file
INIT : load local db config file, isInit = 1
INIT: loading local config file again
GET: invalid dbname input for getDbInst()
GET: invalid dbname input for getDbId()
GET: invalid dbname input for getDbSock()
GET: invalid dbname input for getDbHostname()
GET: invalid dbname input for getDbPort()
testing APPL_DB
redis#0#/var/run/redis/redis.sock#127.0.0.1#6379
testing ASIC_DB
redis1#1#/var/run/redis/redis1.sock#127.0.0.1#6380
testing CONFIG_DB
redis2#4#/var/run/redis/redis2.sock#127.0.0.1#6381
testing COUNTERS_DB
redis1#2#/var/run/redis/redis1.sock#127.0.0.1#6380
testing FLEX_COUNTER_DB
redis3#5#/var/run/redis/redis3.sock#127.0.0.1#6382
testing LOGLEVEL_DB
redis2#3#/var/run/redis/redis2.sock#127.0.0.1#6381
testing PFC_WD_DB
redis3#5#/var/run/redis/redis3.sock#127.0.0.1#6382
testing SNMP_OVERLAY_DB
redis4#7#/var/run/redis/redis4.sock#127.0.0.1#6383
testing STATE_DB
redis4#6#/var/run/redis/redis4.sock#127.0.0.1#6383
[       OK ] DBConnector.multi_db_test (1 ms)

@dzhangalibaba
Copy link
Contributor Author

for each throw can you also add swss_log_error message to make mark in syslog if on someplace someone will forget to catch/log catched exception?

for each throw can you also add swss_log_error message to make mark in syslog if on someplace someone will forget to catch/log catched exception?

Yes. Added.

EXPECT_FALSE(SonicDBConfig::isInit());


// load nonesist config file, should throw exception with file NO exist
Copy link
Contributor

@qiluo-msft qiluo-msft Dec 4, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nonesist [](start = 12, length = 8)

nonexisting_file #Closed

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DONE

Copy link
Contributor

@qiluo-msft qiluo-msft left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor issue.

@dzhangalibaba
Copy link
Contributor Author

Minor issue.

FIXED

@qiluo-msft qiluo-msft merged commit eb01b68 into sonic-net:master Dec 4, 2019
@dzhangalibaba dzhangalibaba deleted the p2_error branch December 4, 2019 22:21
abdosi pushed a commit that referenced this pull request Jan 3, 2020
…#319)

* [MultiDB]: throwing exception when database_config.json doesn't exist and using at() to get map entry
* update file name
* address code format
* add UT and logs
* fix typo
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants