cloud-init-dev team mailing list archive
-
cloud-init-dev team
-
Mailing list archive
-
Message #02088
Re: [Merge] ~chad.smith/cloud-init:cyaml-loading into cloud-init:master
On Fri, May 5, 2017 at 8:21 AM, Scott Moser <smoser@xxxxxxxxxx> 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@xxxxxxxxxxxxxxxxxxx
> 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.
Follow ups
References