onyxframework / onyx

Powerful framework for modern applications 💪
https://api.onyxframework.com/onyx
MIT License
78 stars 5 forks source link

`Onyx::SQL.transaction` must not affect the repo #18

Open vladfaust opened 5 years ago

vladfaust commented 5 years ago
2.times do
  spawn do
    Onyx::SQL.transaction do |tx|
      sleep(rand)
      Onyx::SQL.query # Which tx is it using now – the first or the second one?
    end
  end
end

Proposed API:

Onyx::SQL.transaction do |tx|
  repo = Onyx::SQL.repo.dup
  repo.db = tx # Should patch Onyx::SQL for that
  repo.query() # Instead of Onyx::SQL.query
  # The tx is commited in the end as before
end
repomaa commented 5 years ago

how about:

Onyx::SQL.transaction do |tx|
  tx.query(...)
end

tx wouldn't be a DB::Transaction in this case but rather the duplicated repo with db set to the DB::Transaction. This way it would mask the internals from the API users and it'd be quite intuitive to use.

repomaa commented 5 years ago

Is this Onyx::SQL.transaction method currently implemented? I didn't find anything in the onyx-sql repo

vladfaust commented 5 years ago

@repomaa the definition is here: https://github.com/onyxframework/onyx/blob/9402f0a81bf911818300bcf35bdfc0b652837420/src/onyx/sql.cr#L47

repomaa commented 5 years ago

hm, any reason why it isn't defined in the Onyx::SQL repo?

vladfaust commented 5 years ago

@repomaa yes, because onyx/onyx is a higher-level framework, and onyx/sql is an ORM