Code review comment for lp:~salgado/launchpad/use-meliae

Revision history for this message
Guilherme Salgado (salgado) wrote :

On Thu, 2010-03-18 at 20:42 +0000, Francis J. Lacoste wrote:
> On March 18, 2010, Guilherme Salgado wrote:
> > On Thu, 2010-03-18 at 19:48 +0000, Francis J. Lacoste wrote:
> > > Review: Needs Information
> > > * Don't we need to add meliae to the buildout?
> >
> > We could, but since we already have it packaged in Lucid (I just
> > backported the package to Karmic and Hardy) and the release tarball is
> > not eggified, I thought I'd go with the easiest option. After all, if
> > we need a newer version we can easily switch to using an egg
>
> Well, it does slow down the deployment of this, since they SA will have to
> review/rebuild the package for Hardy (they don't deploy from our PPA) and
> install it on edge/staging before you can land this.
>
> By not eggified, you mean it doesn't have a setup.py?

The problem was that we can't seem to use buildout to make an egg out of
it because it uses pyrex/cython. After discussing this with Gary we came
to the conclusion that the least painful option would be to provide the
eggs ourselves, for now.

>
> > .
> >
> > > * Can we use a configurable file instead of an hardcoded one?
> >
> > I considered that, but I couldn't come up with any use cases that would
> > require changing the file path. I also try to use config values only
> > for things that change between environments -- which doesn't seem to be
> > the case here.
>
> Well, a hardcoded path under /tmp is vulnerable to symlink attacks
> (overwriting of arbitrary files owned by the user).

To exploit that one would need access to the file system, right, in
which case they could just as easily read the hard-coded value from the
config (if we were using a config value). Anyway, I'm willing to move
it to a config variable if you feel strong about it.

--
Guilherme Salgado <email address hidden>

« Back to merge proposal