[PATCH 1/3] Start moving code from the controller to the logic

Bohuslav Kabrda bkabrda at redhat.com
Wed Jan 23 12:25:56 UTC 2013


----- Original Message -----
> On Wed, 2013-01-23 at 06:58 -0500, Bohuslav Kabrda wrote:
> > - I tend to add "user" (who is doing the operation) as argument to
> > all
> > *Logic methods, since this may be needed for some authorization
> > checking etc. Although your methods don't need that, would you
> > please
> > add the argument for consistency?
> > - I would prefer seeing db.session.commit() in views, not *Logic
> > methods. The reason is that if you're doing three logic functions
> > and
> > each one of them commits, it's three separate DB queries - I don't
> > think we want that. That might turn out to be slow (and also with
> > single commit, you either do the _whole_ transaction or nothing of
> > it).
> 
> Will do (for both).
> 
> > - The add_build method should use flask.g.user as a build submitted
> > to
> > be more general and usable from other views, too.
> 

Sorry, that's totally malformed :)
The add_build method should not use copr "owner" for the person who submitted the build, but flask.g.user (it should receive that as a second argument after "cls", as mentioned in my first point). Of course flask.g.user will be passed from the view, the Logic method will know nothing about where the user object came from.
E.g.:

+    def add_build(cls, user, pkgs, copr):
+        build = models.Build(
+            pkgs=pkgs,
+            copr=copr,
+            repos=copr.repos,
+            chroots=' '.join(map(
+                lambda x: x.chroot_name, copr.mock_chroots)
+                ),
+            user=user,
+            submitted_on=int(time.time()))
+        # no need to check for authorization here
+        builds_logic.BuildsLogic.new(user, build, copr,
+            check_authorized = False)

Does this make sense?
(Also, please ignore my bad habit of putting spaces around "=" in function calls. It's not Pythonic and I'm working hard to get rid of it;) )

> I'm not quite sure to see what you have in mind here.
> 
> From a general perspective, I would not "import flask" in the logic,
> as
> I see it the logic should be pretty much a generic python module
> which
> is not linked to the framework (I'm trying to prevent a coupling so
> tight with the framework which might mean that changing framework =
> rewrite the application).
> 
> Pierre
> _______________________________________________
> copr-devel mailing list
> copr-devel at lists.fedorahosted.org
> https://lists.fedorahosted.org/mailman/listinfo/copr-devel
> 

-- 
Regards,
Bohuslav "Slavek" Kabrda.


More information about the copr-devel mailing list