On Fri, May 20, 2011 at 12:58:09PM +0100, Pedro Melo wrote: > Hi, > > On Fri, May 20, 2011 at 3:00 AM, Peter Rabbitson <rabbit+d...@rabbit.us> > wrote: > > On Fri, May 20, 2011 at 11:52:18AM +1000, Toby Corkindale wrote: > >> Hi, > >> I don't know how I failed to pick this up during the testing period > >> for the .19x versions. > >> > >> I have some code that fails on dbic .192, but works when rolled back to > >> .127. > >> The failure is due to the Result class defining some accessors with > >> mk_classdata() which are referred to from within a sqlt_deploy_hook() > >> method. > > > > Known embarrassing regression (I made an incorrect assumption and noone > > caught > > it after the fact). Look forward to 0.08193 by Monday the latest. > > Peter, I was one of the persons who reported this on IRC. > > I said I was going to provide a test case and a patch but I can only > work on non-work stuff on the weekends. I was planning on working on > this on saturday but from your answer I assume that something is > already in the pipeline. > > I'll check the git repo for the latest commits as soon as > git.shadowcat.co.uk is back up on DNS. > > In the meantime, I reverted the commit like this for my personal use: > > https://github.com/melo/dbix-class/commit/ffdb8e2d7e01cc2a8d3b920c43c2d67c6748b988
I think you missed the backlog in-channel earlier. I was actually hoping you guys to take care of it :) [19:51:46] <riba> oh ffs [20:18:12] <riba> http://paste.scsys.co.uk/104045 [20:18:15] <riba> arcanez: ^^ [20:18:21] <riba> melo: ^^ [20:18:37] <riba> please revert this commit, we'll ship a release within a week [20:18:59] * riba fucks off > The important feature for me was making sure that sql_deploy_hook() > chaining, using next::method or next::can, worked, so that I could > have several components using it. > > When I was looking at the code to figure why this had stop working, I > noticed that to a component, the whole sql_deploy_hook() setup is very > fragile. If a resultsource changes the sqlt_deploy_callback(), the > sql_deploy_hook() chain will never be called. Yes I mentioned this in-channel too: [19:44:21] <riba> basically sqlt_deploy_hook is not the correct code-entry [19:44:30] <riba> as in it is not even guaranteed to be called [19:45:07] <riba> sqlt_deploy_callback() is the only thing dbic calls, which by default would call the default_sqlt_deploy_hook which in turn knows how to find the proper sqlt_deploy_hook on the source and call that [19:45:59] <riba> melo: at every point in this chain things are set-able, hence the assumptions you make are far from valid (and I do agree the whole thing is extremely misdesigned) > It would be better for component writers had a hook point that is > always called. Given that it would be a new hook point we can make > sure it has the proper signature. It would be called in > DBIx::Class::ResultSource::_invoke_sqlt_deploy_hook as the last thing > like this: > > my $class = $self->result_class; > $class->new_name_for_new_sql_deploy_hook($self, @_) if $class; > > What do you think? I personally do not like this, but mainly because I hate sqlt_deploy_hook in general - it is not a deploy() hook. It is invoked from within deployment_statements(), which makes the whole thing even more confusing and fragile (i.e. if you do have a ddl-dir already, none of your hooks will ever fire). Furthermore the choice of having the "hooks" live in result classes is down the same alley as ResultSetManager. I would strongly recommend discussing with amiri/mst their experience working with ResultSource (note not Class, Source) componentns, as this is the correct, natural place for such a hook. Note I *am* open to discussion on your earlier proposal, but please understand that what you are proposing is a last-resort architectural compromise, which I really would like to avoid. Cheers _______________________________________________ List: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/dbix-class IRC: irc.perl.org#dbix-class SVN: http://dev.catalyst.perl.org/repos/bast/DBIx-Class/ Searchable Archive: http://www.grokbase.com/group/dbix-class@lists.scsys.co.uk