Re: Repo file generation + tests
----- 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.