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

Fix support for newer Sequel versions #120

Merged
merged 1 commit into from Jun 3, 2012
Merged

Fix support for newer Sequel versions #120

merged 1 commit into from Jun 3, 2012

Conversation

rosenfeld
Copy link
Contributor

Also, there is another catch, I'm not sure how to deal with.

First, I guess the documentation is wrong:

DatabaseCleaner.strategy = :truncation, {:except => %w[widgets]}

Should be:

DatabaseCleaner.strategy = [:truncation, {:except => %w[widgets]}]

But for this to work in Sequel, the tables must be passed as symbols. This is how I use it:

DatabaseCleaner.strategy = [:truncation, except: [:schema_migrations, :roles]]

Sequel will also exclude :schema_info, although I don't use it for keeping track of database changes as I'm using standalone_migrations. This is not a major issue as I don't even have this table, but it could be if that table had a different mean for my application.

I'm only sending a simple patch because I'm not sure how to deal with the other issues.

@bmabey
Copy link
Contributor

bmabey commented Jun 3, 2012

So, I have couple of pull request open for this same issue (#109 solves it the same way actually). However, pull request #121 by @dn uses #adapter_scheme instead. What method is better in this case?

@rosenfeld
Copy link
Contributor Author

Since @jeremyevans is the current maintainer of Sequel itself, better to wait for his answer on the differences between adapter_scheme and database_type :)

The documentation says that database_type is the same as adapter_scheme by default:
http://sequel.rubyforge.org/rdoc/classes/Sequel/Database.html#method-i-database_type

From the explanation I'd guess that database_type is the way to go, but better to wait for his response.

@rosenfeld
Copy link
Contributor Author

Anyway, are there any reasons for not using the code from @jeremyevans above for all database types?

db.from(*tables_to_truncate(db)).truncate

@bmabey
Copy link
Contributor

bmabey commented Jun 3, 2012

Not that I know of. I merged in Sequel support a while ago and I am starting to wonder if the original contributor was very familiar with Sequel since things like this keep popping up. TBH, I know very little about Sequel myself so I'd love to get some direction on this.

@bmabey bmabey mentioned this pull request Jun 3, 2012
@jeremyevans
Copy link

You can't do db.from(*tables_to_truncate(db)).truncate, since most databases do not support truncation of multiple tables at the same time (PostgreSQL does, but Sequel didn't even support that until recently). For determining which type of database you are connecting to, database_type should always be used. adapter_scheme is for when you care which adapter you are using. For example, if you are connecting to PostgreSQL via Sequel's postgres adapter, both database_type and adapter_scheme are :postgres, but if you are connecting to PostgreSQL via Sequel's jdbc adapter, the database_type is :postgres but the adapter_scheme is :jdbc.

In short, the patch in this ticket is correct, and the one in #121 is wrong.

bmabey added a commit that referenced this pull request Jun 3, 2012
Fix support for newer Sequel versions
@bmabey bmabey merged commit adb0d43 into DatabaseCleaner:master Jun 3, 2012
@bmabey
Copy link
Contributor

bmabey commented Jun 3, 2012

Thanks, I've merged this one in.

@rosenfeld
Copy link
Contributor Author

thanks :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants