Remove legacy Rails and Rubies #366

aried3r · 2017-06-26T17:43:30Z

I think even though this gem is kinda-sorta unmaintained (as in, there are no new releases), I think it's time we clean up some stuff.

Things I want to try to do for rpush "4.0" (or 3.0, if we want to add breaking changes there, but I think a lot of people are already using the master branch):

Only support Rails 4.2+
Only support Ruby 2.2+
Drop JRuby support (debatable, it's just that the test always fail, so I have little confidence in it)
Add Appraisals or another way of testing different Rails versions

I'm open for discussion!

Fixes #364

aried3r · 2017-06-26T17:56:09Z

Whoops, accidentally closed it

aried3r · 2017-06-27T09:29:55Z

Currently requiring the whole of Rails to get the tests running. But I think it would be possible to reduce it to active support and active record maybe. First I want a green test run on at least all the active record tests.

I'm not really sure how to tackle the mongoid and redis tests atm. never used those with rpush.

aried3r · 2017-06-28T13:45:15Z

I can't seem to get rpush to work with mongoid for now. Since rpush-mongoid 0.1.0 and 0.2.0 both depend on mongoid versions that do not support Rails 5.x (rather, ActiveModel) I can't seem to get the tests to work.
For example, mongoid 6.2 requires and supports Rails 5.1 https://github.com/mongodb/mongoid/releases/tag/v6.2.0 (actually just actionpack and activemodel)

I'm not sure if the tests heavily depend on Rails in a way that would make them fail, but even now the tests for Rails 4.2 fail. Anyone using rpush-mongoid who can chime in? To use mongoid 5 in the tests we'd have to split the mongoid.yml into two versions because mongoid 5 changed the config file format. For 6 I don't know what is required.

I guess requiring Rails for all the tests makes no sense, so I guess I'd have to split the gemfiles even further in the sense that there'll be "mongoid" gemfiles and "regular" ones.

Otherwise, since we cannot support MongoDB because of rpush-mongoid I'm in favor of dropping support until someone can add support again. That's really harsh, but I'd rather move forward and add support again.
It seems that compared to rpush download numbers rpush-mongoid download numbers are very low.

Directly inheriting from ActiveRecord::Migration is not supported.

This reverts commit 3f9742e.

This reverts commit 1c94d92.

This reverts commit 3509139.

Fivell · 2017-08-31T11:44:45Z

@aried3r nice! any plans to release this to rubygems ?

aried3r · 2017-08-31T12:14:38Z

https://rubygems.org/gems/rpush/versions/3.0.0.rc1 :)

Fivell · 2017-09-07T08:56:08Z

@aried3r , FYI after update rails we have strange error in our specs

Called from /Users/igor/.rvm/gems/ruby-2.3.1/gems/activesupport-5.0.6.rc1/lib/active_support/dependencies.rb:259:in `load_dependency'
[2017-09-07 11:52:42] [rpush] [ERROR] ActiveRecord::ActiveRecordError, Cannot expire connection, it is owned by a different thread: #<Thread:0x007fc0d287f448>. Current thread: #<Thread:0x007fc0dfb26d38>.
/Users/igor/.rvm/gems/ruby-2.3.1/gems/activerecord-5.0.6.rc1/lib/active_record/connection_adapters/abstract_adapter.rb:189:in `expire'
/Users/igor/.rvm/gems/ruby-2.3.1/gems/activerecord-5.0.6.rc1/lib/active_record/connection_adapters/abstract/connection_pool.rb:503:in `block (2 levels) in checkin'
/Users/igor/.rvm/gems/ruby-2.3.1/gems/activesupport-5.0.6.rc1/lib/active_support/callbacks.rb:126:in `call'
/Users/igor/.rvm/gems/ruby-2.3.1/gems/activesupport-5.0.6.rc1/lib/active_support/callbacks.rb:506:in `block (2 levels) in compile'
/Users/igor/.rvm/gems/ruby-2.3.1/gems/activesupport-5.0.6.rc1/lib/active_support/callbacks.rb:455:in `call'
/Users/igor/.rvm/gems/ruby-2.3.1/gems/activesupport-5.0.6.rc1/lib/active_support/callbacks.rb:101:in `__run_callbacks__'
/Users/igor/.rvm/gems/ruby-2.3.1/gems/activesupport-5.0.6.rc1/lib/active_support/callbacks.rb:750:in `_run_checkin_callbacks'
/Users/igor/.rvm/gems/ruby-2.3.1/gems/activerecord-5.0.6.rc1/lib/active_record/connection_adapters/abstract/connection_pool.rb:502:in `block in checkin'
/Users/igor/.rvm/rubies/ruby-2.3.1/lib/ruby/2.3.0/monitor.rb:214:in `mon_synchronize'
/Users/igor/.rvm/gems/ruby-2.3.1/gems/activerecord-5.0.6.rc1/lib/active_record/connection_adapters/abstract/connection_pool.rb:499:in `checkin'
/Users/igor/.rvm/gems/ruby-2.3.1/gems/activerecord-5.0.6.rc1/lib/active_record/connection_adapters/abstract_adapter.rb:459:in `close'
/Users/igor/.rvm/gems/ruby-2.3.1@global/gems/connection_pool-2.2.1/lib/connection_pool.rb:144:in `block in method_missing'
/Users/igor/.rvm/gems/ruby-2.3.1@global/gems/connection_pool-2.2.1/lib/connection_pool.rb:64:in `block (2 levels) in with'
/Users/igor/.rvm/gems/ruby-2.3.1@global/gems/connection_pool-2.2.1/lib/connection_pool.rb:63:in `handle_interrupt'
/Users/igor/.rvm/gems/ruby-2.3.1@global/gems/connection_pool-2.2.1/lib/connection_pool.rb:63:in `block in with'
/Users/igor/.rvm/gems/ruby-2.3.1@global/gems/connection_pool-2.2.1/lib/connection_pool.rb:60:in `handle_interrupt'
/Users/igor/.rvm/gems/ruby-2.3.1@global/gems/connection_pool-2.2.1/lib/connection_pool.rb:60:in `with'
/Users/igor/.rvm/gems/ruby-2.3.1@global/gems/connection_pool-2.2.1/lib/connection_pool.rb:131:in `with'
/Users/igor/.rvm/gems/ruby-2.3.1@global/gems/connection_pool-2.2.1/lib/connection_pool.rb:143:in `method_missing'
/Users/igor/.rvm/gems/ruby-2.3.1/bundler/gems/rpush-2bf846a4d0e4/lib/rpush/daemon/store/active_record.rb:164:in `release_connection'
/Users/igor/.rvm/gems/ruby-2.3.1/bundler/gems/rpush-2bf846a4d0e4/lib/rpush/daemon/feeder.rb:12:in `block in start'

aried3r · 2017-09-07T08:59:29Z

Hmm, I already run rpush 3.0.0 RC1 in production since yesterday after testing for a week in staging. However with Rails 5.0.5, not 5.0.6 RC1.

Do you actively run rpush in the specs or just check if there's new notifications in the database? Because I only test for that, without actually running rpush (since there's no dry-run implemented yet).

Fivell · 2017-09-07T09:45:00Z

@aried3r this can be miss-configuration in our test environment. We found out that this patch resolves this error

require 'rpush'
require 'rpush/daemon/store/interface'
require 'rpush/daemon/store/active_record'
module Rpush
  module Daemon
    module Store
      class ActiveRecord
        def release_connection
         # ::ActiveRecord::Base.connection.close
           ::ActiveRecord::Base.connection_pool.release_connection
        rescue StandardError => e
          Rpush.logger.error(e)
        end
      end
    end
  end

Did you have any issues when stop / restart rpush daemon ?

Fivell · 2017-09-07T09:46:49Z

btw ::ActiveRecord::Base.connection.close was introduced here 5653cab#diff-8c805eff0bcb8b533ecd802f81287ce1L149

aried3r · 2017-09-07T10:09:54Z

Just tried on our staging server. Stopping, starting etc is no problem using 3.0.0.rc1.

That change is three years old and seems not to have been touched since then https://github.com/rpush/rpush/blob/v3.0.0.rc1/lib/rpush/daemon/store/active_record.rb#L164

Can you make sure you're running 3.0.0.rc1? (I see you're pointing to master, which should be the same right now). But I had to shutdown rpush and start it back up again so the daemon used the latest version of rpush. (On that note, rpush status should probably output the rpush version)

I'm afraid I can't really help in this case. It's a Works on my machine!™ type-a-deal :D

But feel free to open a new issue, this PR might not be the best place for others to see.

I also plan to roll out the RC1 as a stable version by next week (wanna test in production some more) so more users will probably update and report bugs (which I hope there aren't any).

Fivell · 2017-09-07T10:33:42Z

@aried3r works for me as well, this lines just logged to log file.
Yes it is 3.0.0.rc1

refs https://github.com/rails/rails/pull/14938/files#diff-c226a4680f86689c3c170d4bc5911e96R149

venriq · 2017-10-11T15:58:01Z

@aried3r I'm a heavy rpush-mongoid user. I can take a look at the gem and update the dependencies. Any specifications in particular?

aried3r · 2017-10-14T15:22:41Z

@venriq, feel free to submit PRs to rpush-mongoid as well as rpush.

See also f1eb5b4 where I removed all of the code that was there before.

Remove legacy Rails and Rubies

aried3r closed this Jun 26, 2017

aried3r reopened this Jun 26, 2017

This was referenced Jun 28, 2017

[REDIS] Rpush::Apns::App.all selects ALL apps (GCM, MPNS, etc.) #325

Closed

Restore JRuby compatibility #241

Closed

aried3r added 23 commits August 31, 2017 12:36

Correct .travis.yml config

9c8fe05

Remove Rails 3 code

1718df9

Remove support for old rubies

d335346

Cleanup .travis.yml, remove rbx, JRuby from CI

018ca7e

Rails 4 removed attr_accessible

a6de637

Use Ruby 2.4.1 locally

26084bd

Update Rails 4.2

d54465b

Use https for github gems

82832f0

Bundle rpush-mongoid in main repo

926e307

Test using Appraisal, drop JRuby on Travis

5b91abd

Move gem dependencies to gemspec

0bbffee

Update gemfiles

4b0ce54

Update mongoid configuration

6ab2603

Use rails, not activerecord in gemfiles

2bdce18

Require rails

6dd812e

Fix migrations for Rails 5.x

b1847ca

Directly inheriting from ActiveRecord::Migration is not supported.

Try Ubuntu 14.04 on Travis CI

ebf7fc7

Bundle rpush-redis in main repo

0bfc7fa

Use bundles rpush-redis

07f1203

Revert "Use bundles rpush-redis"

bb36108

This reverts commit 3f9742e.

Revert "Bundle rpush-redis in main repo"

b2d9722

This reverts commit 1c94d92.

Only require activesupport, but Rails in development

2231da3

Revert "Revert "Bundle rpush-mongoid in main repo"

0c4262a

aried3r added 6 commits August 31, 2017 12:36

Use compatible rpush-modis version

2139f7b

Revert "Update mongoid configuration"

2db07d1

This reverts commit 3509139.

Don't test mongoid on activesupport >4

8f9a1ce

Add require Ruby version to gemspec

a415137

Loosen net-http-persistent dependency for dev

b60ecdc

Remove MongoDB support

f1eb5b4

aried3r force-pushed the ar/remove_legacy_rails branch from 5aea737 to f1eb5b4 Compare August 31, 2017 10:46

aried3r mentioned this pull request Aug 31, 2017

Upgrade modis dep on Gemfile.lock #367

Closed

aried3r added 3 commits August 31, 2017 13:10

Update changelog

03be693

Allow Ruby 2.2.2 to be installed

1389b4d

Add migration advice to CHANGELOG [ci skip]

5761ffe

aried3r merged commit 2fa09cc into rpush:master Aug 31, 2017

aried3r deleted the ar/remove_legacy_rails branch August 31, 2017 11:27

nattfodd mentioned this pull request Feb 7, 2018

Upgrade from 2.7 to 3.0, application side changes. #406

Closed

Adrian1707 pushed a commit to Adrian1707/rpush that referenced this pull request Apr 24, 2024

Merge pull request rpush#366 from aried3r/ar/remove_legacy_rails

e4f957b

Remove legacy Rails and Rubies

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove legacy Rails and Rubies #366

Remove legacy Rails and Rubies #366

aried3r commented Jun 26, 2017 •

edited

Loading

aried3r commented Jun 26, 2017

aried3r commented Jun 27, 2017

aried3r commented Jun 28, 2017

Fivell commented Aug 31, 2017

aried3r commented Aug 31, 2017

Fivell commented Sep 7, 2017 •

edited

Loading

aried3r commented Sep 7, 2017

Fivell commented Sep 7, 2017

Fivell commented Sep 7, 2017

aried3r commented Sep 7, 2017

Fivell commented Sep 7, 2017 •

edited

Loading

venriq commented Oct 11, 2017 •

edited

Loading

aried3r commented Oct 14, 2017

Remove legacy Rails and Rubies #366

Remove legacy Rails and Rubies #366

Conversation

aried3r commented Jun 26, 2017 • edited Loading

aried3r commented Jun 26, 2017

aried3r commented Jun 27, 2017

aried3r commented Jun 28, 2017

Fivell commented Aug 31, 2017

aried3r commented Aug 31, 2017

Fivell commented Sep 7, 2017 • edited Loading

aried3r commented Sep 7, 2017

Fivell commented Sep 7, 2017

Fivell commented Sep 7, 2017

aried3r commented Sep 7, 2017

Fivell commented Sep 7, 2017 • edited Loading

venriq commented Oct 11, 2017 • edited Loading

aried3r commented Oct 14, 2017

aried3r commented Jun 26, 2017 •

edited

Loading

Fivell commented Sep 7, 2017 •

edited

Loading

Fivell commented Sep 7, 2017 •

edited

Loading

venriq commented Oct 11, 2017 •

edited

Loading