Repo file generation + tests

Bohuslav Kabrda bkabrda at redhat.com
Fri Apr 19 06:18:26 UTC 2013


----- Original Message -----
> On Thu, 2013-04-18 at 14:24 +0200, Tomas Radej wrote:
> > +    try:
> > +        # We're taking the baseurl from any finished build in the
> > given copr.
> > +        # It's ugly, but so far we didn't come up with anything else.
> > +        copr = coprs_logic.CoprsLogic.get(flask.g.user, username,
> > coprname,
> > +                with_mock_chroots=True,
> > with_builds=True).filter(models.Build.results != None).one()
> > +    except NoResultFound:
> > +        if not coprs_logic.CoprsLogic.get(flask.g.user, username,
> > coprname).first():
> > +            return page_not_found('Copr: {0}/{1} does not
> > exist'.format(username, coprname))
> > +        else:
> > +            return page_not_found('Repository not initialized: No
> > builds in {0}/{1} are finished.'.format(username, coprname))
> 
> I don't know how much we're already doing it in the current code so the
> question is more style related.
> Do we want to have sql (ie SQLAlchemy query) in the controller?
> 
> Personally, I'm not fan of it since it means that if one day the ORM
> changes, there is code to change all over the place.
> But that's just my personal preference :)
> 
> Pierre

Good point, Pierre. I'm not quite sure about usage of sqlalchemy in views right now, but I guess it's minimal and it's true that keeping it that way is a good idea. So Tomas, maybe you could modify CoprsLogic.get to accept an argument like ended_builds_only, which would change the with_builds behaviour in a proper way? And this should probably actually return the copr even if no ended builds are found (which would simplify your view code, but will probably require using a subselect query or so).

Thanks,
Slavek.

-- 
Regards,
Bohuslav "Slavek" Kabrda.


More information about the copr-devel mailing list