Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
vladfaust opened this issue Apr 25, 2019 · 5 comments
Open

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

vladfaust opened this issue Apr 25, 2019 · 5 comments
Labels
breaking Breaking change design flaw Works as expected, but overall wrong
Milestone

Comments

@vladfaust
Copy link
Member

vladfaust commented Apr 25, 2019

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
@vladfaust vladfaust added bug Something isn't working breaking Breaking change design flaw Works as expected, but overall wrong and removed bug Something isn't working labels Apr 25, 2019
@vladfaust vladfaust added this to the next minor milestone May 28, 2019
@repomaa
Copy link

repomaa commented Jul 24, 2019

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
Copy link

repomaa commented Jul 24, 2019

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

@vladfaust
Copy link
Member Author

@repomaa the definition is here:

def self.transaction(&block : ::DB::Transaction ->)

@repomaa
Copy link

repomaa commented Jul 24, 2019

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

@vladfaust
Copy link
Member Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking Breaking change design flaw Works as expected, but overall wrong
Projects
None yet
Development

No branches or pull requests

2 participants