Initial empty dictionary for None additional cluster configs in OSD IntegTest #3593
Conversation
Signed-off-by: Peter Zhu <zhujiaxi@amazon.com>
Signed-off-by: Peter Zhu <zhujiaxi@amazon.com>
peterzhuamazon
requested review from
dblock,
bbarani,
gaiksaya,
rishabh6788,
zelinh,
jordarlu,
prudhvigodithi,
Divyaasm and
tianleh
as code owners
peterzhuamazon
changed the title
Codecov Report
@@ Coverage Diff @@
## main #3593 +/- ##
=======================================
Coverage 91.49% 91.49%
=======================================
Files 181 181
Lines 5374 5376 +2
=======================================
+ Hits 4917 4919 +2
Misses 457 457
|
| @@ -84,6 +84,9 @@ def __setup_cluster_and_execute_test_config(self, config: str) -> int: | |||
| self.additional_cluster_config = self.test_config.integ_test.get("additional-cluster-configs") | |||
| logging.info(f"Additional config found: {self.additional_cluster_config}") | |||
|
|
|||
| if self.additional_cluster_config is None: | |||
There was a problem hiding this comment.
how about just initialize self.additional_cluster_config as empty directory and check the len or keys() method to check if it has any values? @peterzhuamazon
There was a problem hiding this comment.
That can be done but right now I am just matching the OS side implementation.
We can improve this in later PRs.
There was a problem hiding this comment.
I'm late to this game, but assigning of a default should happen in the constructor, not inside the path of execution because it becomes a surprising side-effect of the latter.
Otherwise do self.additional_cluster_config or {} anywhere it's used.
|
This is also to follow up #3590. |
Description
Initial empty dictionary for None additional cluster configs
Issues Resolved
#3589 partially
#3434
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.