On Fri, May 5, 2017 at 8:21 AM, Scott Moser <smo...@ubuntu.com> wrote:
> > From Ryan: > > Some thoughts on this: > > > > 1) we don't want users who apt install cloud-init to also pull down > python- > > devel, gcc and have to compile the extension, so please don't change > package > > deps here > > I don't follow this. In ubuntu at least cloud-init does depend on > python3-yaml and python3-yaml on libyaml-0-2. > > $ apt-cache show python3-yaml | grep Dep > Depends: python3 (<< 3.6), python3 (>= 3.5~), libc6 (>= 2.14), libyaml-0-2 > > There is no reason to explicitly depend on libyaml-0-2, and we have a > fallback > path in the code. > > Why did you think we need python-devel ? This is where tox falls apart > honestly, when you get into C python extensions. But we already have some > of those in the test path (pylxd). > tox compiling > > > 2) this is currently an issue for the tox/venv environment, so let's > focus on > > how to enable the SafeLoader library for that path > > if we do skipIf, then the package build environment will run though the > CLoader path, and the tox path would not. We could put in some way of > failing if the package build environment didn't take the CLoader path. > That'd ensure that we were getting tests on both paths. > > I think that might be over-engineering. > > > 3) Once we make (2) an optional testing path (env or parameter can > trigger the > > dep install prior to calling tox/venv which triggers the compile during > pip > > install of the module IIUC), we can have two travis paths, one without > the > > library (representing how things work today) and one *with* the library > > enabled. This allows us to compare results, unittests all pass, and time > it > > takes to complete (some estimate of the improvement it may provide). > > > > 4) With results from (3), we may propose changes to the underlying images > > (server seed, cloud-image seed, etc) which could ensure that the library > is > > present in the default cloud images. > > I think over-engineering, especially for the size of the gain. Its > entirely possible that there is overhead of loading c that makes this path > slower, meaning we'd be doing this work for negative result. > > Best case, i think the amount of yaml that cloud-init loads is probably > small enough to look like noise here. > Ideally we'll generate some actual numbers here; if it's not worth it, then we can dump the whole thing. If it is, ideally we'd have a path through both w.r.t tox/unittesting; I can't say that it's over-engineering to want to exercise both paths we might expect cloud-init users might execute. > > > -- > https://code.launchpad.net/~chad.smith/cloud-init/+git/ > cloud-init/+merge/323088 > Your team cloud init development team is requested to review the proposed > merge of ~chad.smith/cloud-init:cyaml-loading into cloud-init:master. > > _______________________________________________ > Mailing list: https://launchpad.net/~cloud-init-dev > Post to : cloud-init-dev@lists.launchpad.net > Unsubscribe : https://launchpad.net/~cloud-init-dev > More help : https://help.launchpad.net/ListHelp > -- https://code.launchpad.net/~chad.smith/cloud-init/+git/cloud-init/+merge/323088 Your team cloud init development team is requested to review the proposed merge of ~chad.smith/cloud-init:cyaml-loading into cloud-init:master. _______________________________________________ Mailing list: https://launchpad.net/~cloud-init-dev Post to : cloud-init-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~cloud-init-dev More help : https://help.launchpad.net/ListHelp
