thoughtbot / griddler

Simplify receiving email in Rails (deprecated)
http://griddler.io/
MIT License
1.38k stars 199 forks source link

A copy of EmailProcessor has been removed from the module tree but is still active! #170

Closed nambrot closed 9 years ago

nambrot commented 10 years ago

This is on a brand new Rails install (admittedly 4.2.beta1, but it's not Spring related).

All default settings, an EmailProcessor that just prints out a debug message. When the server is running and I modify the EmailProcessor, it throws that error.

calebhearth commented 10 years ago

Interesting. Running any exotic Ruby version?

Would a fresh 4.2.beta1 app with Griddler installed do this on startup? Only when the file is modified?

nambrot commented 10 years ago

Ruby 2.1.1 A fresh 4.2.beta1 install indeed ᐧ

On Fri, Aug 29, 2014 at 10:35 AM, Caleb Thompson notifications@github.com wrote:

Interesting. Running any exotic Ruby version?

Would a fresh 4.2.beta1 app with Griddler installed do this on startup? Only when the file is modified?

— Reply to this email directly or view it on GitHub https://github.com/thoughtbot/griddler/issues/170#issuecomment-53883776.

calebhearth commented 10 years ago

Cool. Investigating, but it seems likely that this patch will fix it:

diff --git a/lib/griddler/configuration.rb b/lib/griddler/configuration.rb
index 615cb98..5e01499 100644
--- a/lib/griddler/configuration.rb
+++ b/lib/griddler/configuration.rb
@@ -22,7 +22,7 @@ module Griddler
       @processor_class ||=
         begin
           if Kernel.const_defined?(:EmailProcessor)
-            EmailProcessor
+            ::EmailProcessor
           else
             raise NameError.new(<<-ERROR.strip_heredoc, 'EmailProcessor')
               To use Griddler, you must either define `EmailProcessor` or configure a
nambrot commented 10 years ago

Just ran it quickly, but the error still persists

nambrot commented 10 years ago

I assume what happens is that configuration does not get reloaded with Rails auto-reloading mechanism, and since we store the actual reference in @processor_class, it will refer to an outdated version?

Bypassing the memoization and just returning EmailProcessor avoids the error

calebhearth commented 10 years ago

I assume what happens is that configuration does not get reloaded with Rails auto-reloading mechanism, and since we store the actual reference in @processor_class, it will refer to an outdated version?

Yeah, that's the issue I believe.

Bypassing the memoization and just returning EmailProcessor avoids the error

Not an ideal solution, but possibly a good idea for you in the short term. Realistically with low throughput it probably doesn't matter.

This seems related as well. https://github.com/activeadmin/activeadmin/issues/2334

nambrot commented 10 years ago

How about switching depending on environment or worst case a config var? On Aug 29, 2014 12:05 PM, "Caleb Thompson" notifications@github.com wrote:

I assume what happens is that configuration does not get reloaded with Rails auto-reloading mechanism, and since we store the actual reference in @processor_class, it will refer to an outdated version?

Yeah, that's the issue I believe.

Bypassing the memoization and just returning EmailProcessor avoids the error

Not an ideal solution, but possibly a good idea for you in the short term. Realistically with low throughput it probably doesn't matter.

This seems related as well. activeadmin/activeadmin#2334 https://github.com/activeadmin/activeadmin/issues/2334

— Reply to this email directly or view it on GitHub https://github.com/thoughtbot/griddler/issues/170#issuecomment-53895933.

calebhearth commented 9 years ago

Fixed in #170

rdetert commented 9 years ago

I'm still seeing this with:

Rails 4.1.9 Ruby 2.0.0-p481 griddler (1.1.0)