I''ve written a patch/solution for issues I''m having with the transaction system [ongoing in http://groups.google.com/group/rubyonrails-core/browse_thread/thread/344b6f25b19e2a35] while going through the tests one of the tests caught me out. at the moment it appears that the following: SomeModel.transaction do @item1.save break @item2.save end Will result in a successfully saved @item1 record. This seems counter intuitive to the concept of the transaction where "everything must succeed" Can I propose (and include in my patch) that this functionality be altered so that method returns / breaks do NOT cause commits? --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To post to this group, send email to rubyonrails-core@googlegroups.com To unsubscribe from this group, send email to rubyonrails-core-unsubscribe@googlegroups.com For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en -~----------~----~----~----~------~----~------~--~---
On Mon, February 26, 2007 11:37 am, chrisfarms wrote:> at the moment it appears that the following: > > SomeModel.transaction do > @item1.save > break > @item2.save > end > > Will result in a successfully saved @item1 record. > > This seems counter intuitive to the concept of the transaction where > "everything must succeed" > > Can I propose (and include in my patch) that this functionality be > altered so that method returns / breaks do NOT cause commits?I suppose it depends on what you consider to be a normal flow of control vs an exceptional flow. What if you re-wrote your code as the following? SomeModel.transaction do @item1.save if some_condition @item2.save end end I''d think it would be proper for item1 to be saved but not item2. So how different is that from doing a break or return in a transaction block? -- Josh Susser http://blog.hasmanythrough.com --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To post to this group, send email to rubyonrails-core@googlegroups.com To unsubscribe from this group, send email to rubyonrails-core-unsubscribe@googlegroups.com For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en -~----------~----~----~----~------~----~------~--~---
hey Josh.. I like the syntax as you described as in my mind the block is doing this: "until I say GO just try to do the following" try and save @item1 try and save @item2 if some_condition "ok thats great GO make that permanent" whereas what I described it is much more ambiguous. using break/return- from-method don''t imply either a successful OR a failed transaction. I think it''s this slight confusion that worries me.... if it could have two possible interpretations, then I would hope that the less dangerous one would be implemented? I''d certainly prefer to find out that the reason my model wasn''t being destroyed was that I can''t COMMIT a transaction with a ''return'', than to discover that my data is going missing because I was using break to drop outs of a transaction early? On Feb 26, 8:09 pm, "Josh Susser" <j...@hasmanythrough.com> wrote:> On Mon, February 26, 2007 11:37 am, chrisfarms wrote: > > at the moment it appears that the following: > > > SomeModel.transaction do > > @item1.save > > break > > @item2.save > > end > > > Will result in a successfully saved @item1 record. > > > This seems counter intuitive to the concept of the transaction where > > "everything must succeed" > > > Can I propose (and include in my patch) that this functionality be > > altered so that method returns / breaks do NOT cause commits? > > I suppose it depends on what you consider to be a normal flow of control > vs an exceptional flow. What if you re-wrote your code as the following? > > SomeModel.transaction do > @item1.save > if some_condition > @item2.save > end > end > > I''d think it would be proper for item1 to be saved but not item2. So how > different is that from doing a break or return in a transaction block? > > -- > Josh Susserhttp://blog.hasmanythrough.com--~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To post to this group, send email to rubyonrails-core@googlegroups.com To unsubscribe from this group, send email to rubyonrails-core-unsubscribe@googlegroups.com For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en -~----------~----~----~----~------~----~------~--~---
> I think it''s this slight confusion that worries me.... if it could > have two possible interpretations, then I would hope that the less > dangerous one would be implemented? > > I''d certainly prefer to find out that the reason my model wasn''t being > destroyed was that I can''t COMMIT a transaction with a ''return'', than > to discover that my data is going missing because I was using break to > drop outs of a transaction early?It''s way too risky for rails to try and guess what your intentions are. That way lies madness You can do this at present by raising an exception to roll back the transaction, but you then have to rescue it so at present you have to do begin transaction do @item.save raise SomethingError.new end rescue SomethingError=>se end if we could simplify that to transaction do @item.save raise ActiveRecord::RollbackTransaction.new end Life would be much easier for all concerned Alternatively in edge you can currently rollback the transaction explicitly. http://dev.rubyonrails.org/changeset/6196 That API isn''t necessarily the final one, but it would certainly be nice to have an exception you can raise to roll back the transaction without necessarily having to rescue it explicitly. -- Cheers Koz --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To post to this group, send email to rubyonrails-core@googlegroups.com To unsubscribe from this group, send email to rubyonrails-core-unsubscribe@googlegroups.com For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en -~----------~----~----~----~------~----~------~--~---
The problem with the current system is that
@item = Item.new
begin
transaction do
@item.save
raise
end
rescue
end
@item.new_record? == false
I think the syntax for your raise way would be better to keep a system
like in edge where rollback! and rollback act differently
transaction do |c|
c.rollback!
puts ''this doesnt happen''
end
transaction do |c|
c.rollback
puts ''carries on''
end
easy to add on the current system too.....
I''ve been having fix-off today... [PATCH] that fixes the first problem
I mentioned ... though that did involve some magic :)
http://dev.rubyonrails.org/ticket/7658
would love some feedback on the ideas.
cheers
farms.
On Feb 27, 12:57 am, "Michael Koziarski" <mich...@koziarski.com>
wrote:> > I think it''s this slight confusion that worries me.... if it
could
> > have two possible interpretations, then I would hope that the less
> > dangerous one would be implemented?
>
> > I''d certainly prefer to find out that the reason my model
wasn''t being
> > destroyed was that I can''t COMMIT a transaction with a
''return'', than
> > to discover that my data is going missing because I was using break to
> > drop outs of a transaction early?
>
> It''s way too risky for rails to try and guess what your intentions
> are. That way lies madness
>
> You can do this at present by raising an exception to roll back the
> transaction, but you then have to rescue it
>
> so at present you have to do
>
> begin
> transaction do
> @item.save
> raise SomethingError.new
> end
> rescue SomethingError=>se
> end
>
> if we could simplify that to
>
> transaction do
> @item.save
> raise ActiveRecord::RollbackTransaction.new
> end
>
> Life would be much easier for all concerned
>
> Alternatively in edge you can currently rollback the transaction
explicitly.
>
> http://dev.rubyonrails.org/changeset/6196
>
> That API isn''t necessarily the final one, but it would certainly
be
> nice to have an exception you can raise to roll back the transaction
> without necessarily having to rescue it explicitly.
>
> --
> Cheers
>
> Koz
--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com
To unsubscribe from this group, send email to
rubyonrails-core-unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en
-~----------~----~----~----~------~----~------~--~---