Extract Firebird / Sqlserver / Oracle database tasks, and They should be deprecated. #9971

Merged
merged 8 commits into from Apr 2, 2013

5 participants

@kennyj

We'll introduce new database_tasks.rb and register tasks api since rails 4.0.

Thus, I think we should extract and deprecate firebird / sqlserver / oracle database tasks because they should be supported by 3rd-party adapters.

/cc @rafaelfranca @carlosantoniodasilva @yahonda

@rafaelfranca
Ruby on Rails member

👍 @yahonda any objections?

@rafaelfranca
Ruby on Rails member

@tenderlove what do you think?

@vipulnsward
Ruby on Rails member

this is cool!

@yahonda

Will take a look at this.

cc / @metaskills I think you maintain sql server adapter.

@metaskills

Fine by me, we have always had to hack around these anyway. Would much rather prefer the new proposed API.

@metaskills metaskills referenced this pull request in rails-sqlserver/activerecord-sqlserver-adapter Mar 28, 2013
Closed

Active Record 4.0.0 not supported #250

@yahonda

Hi, I've merged these commits into my sandbox branch and tested. Every database adapter (sqlite, mysql, mysql2, postgresql and oracle) this test gets error.

  1) Error:
ActiveRecord::MySQLStructureDumpTest#test_warn_when_external_structure_dump_fails:
TypeError: can't convert Tempfile into StringIO
    /home/yahonda/git/rails/activesupport/lib/active_support/core_ext/kernel/reporting.rb:87:in `reopen'
    /home/yahonda/git/rails/activesupport/lib/active_support/core_ext/kernel/reporting.rb:87:in `capture'
    /home/yahonda/git/rails/activerecord/test/cases/tasks/mysql_rake_test.rb:277:in `test_warn_when_external_structure_dump_fails'

Here are all outputs.
https://gist.github.com/yahonda/5270973 I'm okay to merge this pull request if these errors resolved.

@kennyj

@yahonda Thank you for testing ! I can reproduce above error, and I'm going to fix it.

@kennyj

I've updated commits to fix the above error. I used capture method instead of swapping $stderr in testcase

@yahonda

Thanks for the update. All errors have gone.

Is this an expected behavior to show deprecation messages every time, rake test_sqlite3 and other bundled adapters are tested?

https://gist.github.com/yahonda/5272825

@rafaelfranca
Ruby on Rails member

I don't think so.

@kennyj

@yahonda thanks for your comment. I've confirmed it. I'm going to check one tonight. Now I'm working on holiday ;)

@kennyj

@yahonda @rafaelfranca I've udpated deprecation warning problem. I added 2b32b0597ab082404c9146892912c31014c075a0.

I thought no D.R.Y., but above code will be removed. So I left duplicated code alone.

@yahonda

@kennyj Thanks for making a fix.

2b32b05 mutes all deprecated messages not only for bundled adapters (sqlite3, mysql, mysql2 and postgresql) but for Oracle enhanced adapter. I do not have SQLServer or Firebird database environments, it should mute deprecate messages for these adapters also.

What I meant to say was this deprecate message should appear when Oracle, Firebird and SQLServer adapters are tested. but it should not appear when bundled adapters (sqlite3, mysql, mysql2 and postgresql) are tested.

I was trying to make a fixusing current_adapter? but actually these tests use Mocha::Mock adapter, then made a commit using if adapter names are defined or not.

Here are both outputs.
rake test_oracle output and rake test output

This output is what I expected whatever the way it is implemented.

@kennyj

@yahonda thanks. I agree with your commit.
I want to remove 2b32b05
and add your commit . But your commit depend on my commit .
Could you provide a commit without dependency ?
or should I fix my commit ?

@yahonda

Yes my commit needs your 2b32b05. I do not think you need to revert/remove 2b32b05. I think it should be fine your pull request includes yahonda@beff312. Would you cherry pick yahonda@beff312 and update your pull request?

@kennyj

@yahonda ok. I'm going to include your commit. Thanks :)

@kennyj

I've updated my PR again :) I included @yahonda 's commit.

@rafaelfranca
Ruby on Rails member

Great @kennyj. Now we need a CHANGELOG entry

@kennyj

@rafaelfranca I've added a CHANGELOG entry .

@rafaelfranca
Ruby on Rails member

@kennyj could you squash these two commits? kennyj@24e093f kennyj@2f33670

@kennyj

@rafaelfranca I've updated again :)

@rafaelfranca rafaelfranca merged commit f95111e into rails:master Apr 2, 2013
@rafaelfranca
Ruby on Rails member

Thank you

@kennyj

Thanks guys !

@kennyj kennyj deleted the kennyj:refactor_tasks branch Apr 2, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment