Config precheck - Githubissues

CCI-MOC / hil

Hardware Isolation Layer, formerly Hardware as a Service

Apache License 2.0

24 stars 54 forks source link

Config precheck #966

Closed ianballou closed 6 years ago

ianballou commented 6 years ago

Addresses #45:

Config file is now checked when the HIL server is started
Sections that are not critical to HIL are optional
Option types are checked as well, such as strings vs bools
Tests that didn't include all of the required HIL.cfg sections/options have been updated

Note: I am going to start working on testing, but I wanted to get this out for review first since I'll be out next week.

coveralls commented 6 years ago

Pull Request Test Coverage Report for Build 1797

32 of 60 (53.33%) changed or added relevant lines in 8 files are covered.
1 unchanged line in 1 file lost coverage.
Overall coverage increased (+55.6%) to 55.559%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
hil/ext/switches/nexus.py	0	4	0.0%
hil/ext/switches/brocade.py	0	4	0.0%
hil/ext/switches/dell.py	0	4	0.0%
hil/ext/switches/n3000.py	0	4	0.0%
hil/ext/switches/dellnos9.py	0	4	0.0%
hil/config.py	28	36	77.78%
<!--	Total:	32	60	53.33%	-->

Files with Coverage Reduction	New Missed Lines	%
hil/config.py	1	73.97%
<!--	Total:	1		-->

Totals
Change from base Build 1796:	55.6%
Covered Lines:	2049
Relevant Lines:	3688

💛 - Coveralls

xuhang57 commented 6 years ago

Officially the first PR which increases the coverage 💟 🍸 🤙

ianballou commented 6 years ago

I believe I addressed the comments above. @zenhack for more in-depth config checks, I've added checks for malformed web/db urls, a check for the VLAN pool list, and a check for log_level. Naved and I discussed checking the headnode config options, and we decided they can be left as str for now since they will be removed in the future.

I considered checking log_dir, however a bad directory crashes HIL before before the config is even loaded. I'll add an issue about that.

ianballou commented 6 years ago

Tests will be on the way as soon as we lock in which config checks should be there.

ianballou commented 6 years ago

Tests are in progress now.

ianballou commented 6 years ago

@naved001 I addressed your questions in this commit I believe.

ianballou commented 6 years ago

I've updated the functions to better use the schema library. I tried to not use lambdas, but I went with them since it seems to be a standard in the schema docs.

I also fixed up the IOError checking. In testing, EACCES was raised rather than EPERM for directory permissions. In case of an unexpected error, I re-raise it so the CLI can deal with it.

ianballou commented 6 years ago

Everything should be all set now.

zenhack commented 6 years ago

LGTM, merging.