Skip to content

Conversation

mecampbellsoup
Copy link
Contributor

@mecampbellsoup mecampbellsoup commented Nov 21, 2016

Current Behavior

Does not work with Rails 3 due to gemspec versioning of the rails dependency.

Why Change?

We'd like to use this gem within zipmark-service which is a Rails 3 application.

Implementation

  • lower rails version dependency from 4.2 to 3.2 in the gemspec
  • only evaluate ActiveRecord::Migration::CheckPending.class_eval block if Rails version is >= 4

* lower `rails` version dependency from 4.2 to 3.2
* skip `CheckPending.class_eval` unless Rails version is >= 4
@mecampbellsoup
Copy link
Contributor Author

@jakehow @kurko please review (moved from #1)

@kurko
Copy link

kurko commented Nov 21, 2016

Looks good-ish. This lib has no tests though.

Could you give me a bit more context into this? It's called allow_connection_failure, does it mean that if we get a genuine database connection error, we will ignore them (as if the app had no DB in the first place)?

What apps are using this lib?

@jakehow
Copy link
Member

jakehow commented Nov 21, 2016

@kurko AR code in Rails doesn't require an app to even start successfully. We want to be able to start our apps even in the absence of the database, and then respond to endpoints which do not require DB access successfully. Primarily this is typically our status endpoints. We want to be able to report that the container, and the app are up, but that the DB is down.

If you actually try to execute any endpoints or other codepaths that require DB access you should still get an exception.

@jakehow
Copy link
Member

jakehow commented Nov 21, 2016

@kurko @mecampbellsoup we should put this in the readme maybe? Associated rails issue -> rails/rails#22154

@kurko
Copy link

kurko commented Nov 22, 2016

That makes sense.

@mecampbellsoup
Copy link
Contributor Author

@kurko @jakehow updated README and PR description

@mecampbellsoup
Copy link
Contributor Author

@kurko @jakehow GTG?

@jakehow
Copy link
Member

jakehow commented Nov 23, 2016

@mecampbellsoup while i have no idea how to add the rails tests we could easily add dummy apps for the supported rails versions to confirm this is working from the user's perspective

@jakehow
Copy link
Member

jakehow commented Nov 23, 2016

@mecampbellsoup also you can run the zipmark-service tests with this branch included to make sure it doesnt break anything

@mecampbellsoup
Copy link
Contributor Author

@jakehow as, say, fixtures at spec/fixtures/rails-3, rails-4 and now also rails-5 perhaps?

Should it be a rake task or something, e.g. rake test_with_rails_version 3?

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.

3 participants