← Back to team overview

cloud-init-dev team mailing list archive

Re: [Merge] ~kaihuan-pkh/cloud-init:aliyun-datasource into cloud-init:master

 

Hi,

this looks mostly good, a few small things.


 * please write a "combined" commit message and add it here by hitting 'Set commit message'.  That is what i'll use when i squash and pull your change.
 * some flake8 issues still, easily see with 'tox -e flake8'.
   fixes are trivial.
 * parse_public_keys: move this to a stand alone method, it does not use 'self' at all.  Then please add some more tests.
   seems you're missing a test for if it is a dictionary of dictionaries.
 * what do 'get_ntp_conf' and 'source_address' do?
   it doesn't look like they're used anywhere.  I'd rather drop them unless they're needed.
 * wait_for_metadata_service: this doesnt look necessary.
   the only differences between it and the ec2 version is a check for
     if not mcfg:
         mcfg = {}
   I'm pretty sure that self.dscfg should be a dictionary always.
 * DEF_MD_VERSION = "latest"
   is there a api version that we can use specifically rather than 'latest'?
   Almost by definition, 'latest' is not something you should depend on
   behavior in.  In EC2, we use a specific version because we expect
   the data there to always be per that "api".  Using 'latest' will break
   us if the metadata server ever changes.
 * you have some code like this in a few places that I don't think is
   necessary.  self.metadata should always contain a dictionary
   after it has had 'get' done on it.
     if not self.metadata:
            return {}

-- 
https://code.launchpad.net/~kaihuan-pkh/cloud-init/+git/cloud-init/+merge/308483
Your team cloud init development team is requested to review the proposed merge of ~kaihuan-pkh/cloud-init:aliyun-datasource into cloud-init:master.


References