cloud-init-dev team mailing list archive

[Chad Smith <chad.smith@xxxxxxxxxxxxx>]

 Thanks for the clarification and background on the datasource list ordering. And I see your point on git log (as that's how I determined what the intent of the var was in the first place). 


+1 on the separate mocked unit test for _is_aliyun() to exercise the interface we are actually calling. It'd be nice at some point if we had a fake dmidecode or SMBIOS test resource which could be used to seed datasource testing. Then we wouldn't really have to 'mock', just inject required dmi values and let things run.

Per the importing the ay.ALIYUN_PRODUCT variable into unit tests, I agree with your consistency comment. Having a module variable imported into that module's unit tests only asserts that the module consistently references whatever the variable's value is (correct or incorrect).  I'd suggest though that copying that string directly into the unit test does the same type of conisitency validation. The module writer could have defined a variable which won't work on aliyun systems in the first place, and then tested that  ALIYUN_PRODUCT == "blah" as well.  It doesn't actually assert that this value will work on aliyun environments. We'd probably have to rely on integration tests for that.

Also this suggestion was to try to also bridge some consistency between our cloudinit/source/DataSourceAliyun.py and ds-identify. It feels like things are somewhat shallow in the consistency checks between tools/ds-identify logic and datasource detection logic in  cloudinit/source/DataSource*py. It's probably due in part to the transition to strict ds validation etc. In a perfect world all this common logic to parse DMI data or metadata could be in one place, but I realize ds-identify has some constraints on where it is run (and speed) so loading up python modules probably doesn't make sense.
-- 
https://code.launchpad.net/~smoser/cloud-init/+git/cloud-init/+merge/324625
Your team cloud-init commiters is requested to review the proposed merge of ~smoser/cloud-init:feature/enable-aliyun into cloud-init:master.