Make Dynamoid Rails 5 compatible and help avoid errors when no tables exist yet #63

adamthedeveloper · 2016-03-30T05:13:41Z

The nil object doesn't have an include? method. In case the tables variable is nil, default it to an empty array so the .include? works.

Remove the inclusion of ActiveModel::Serializers::Xml from components.rb. It's throwing errors and from what I can tell, it's not used anywhere in the codebase. Check my search though: https://github.com/Dynamoid/Dynamoid/search?q=XML&type=Code&utf8=%E2%9C%93

After making these tweaks, I was able to successfully run this gem in Rails 5 without any initial tables.

allow this gem to run on rails 5

adding the active_model_serializers runtime dependency

In an effort to eliminate the XML errors I was seeing while running this in rails 5, I added active-model-serializers-xml as a runtime dependency. It changed nothing, I was still getting those errors.

This was throwing errors stating that ActiveModel::Serializers::XML couldn't be found. After removing it, everything seemed to work fine for me. What is it being used for?

coveralls · 2016-03-30T09:02:28Z

Coverage decreased (-1.4%) to 94.174% when pulling efe9e6c on adamthedeveloper:master into 1db4779 on Dynamoid:master.

pcorpet · 2016-03-30T20:37:46Z

Can you add a test that would highlight the original problem please?

adamthedeveloper · 2016-03-30T21:11:19Z

sure. If I get some time. It may be a little while.

cavi21 · 2016-04-10T19:31:58Z

lib/dynamoid/adapter.rb

@@ -19,7 +19,7 @@ def tables
      if !@tables_.value
        @tables_.swap{|value, args| benchmark('Cache Tables') {list_tables}}
      end
-      @tables_.value
+      @tables_.value || []


Hey everyone! I'm also looking to use Dynamoid with Rails 5 and so I fork it and apply all this changes and started to build a test to cover this situation (FYI I did not experience this issue though). But looking into the code I got confuse with this change, because the

if !@tables_.value @tables_.swap{|value, args| benchmark('Cache Tables') {list_tables}} end

should do exactly the same as this @tables_.value || [] because the output of the method list_tables of the Dynamoid::AdapterPlugin::AwsSdkV2 is an empty array. So I imagine that's this is depending at the end on the Aws::DynamoDB::Client version that you are using, because the Dynamoid::AdapterPlugin::AwsSdkV2 send that call to the client that it's a Aws::DynamoDB::Client.

Bottom line the response of the Aws::DynamoDB::Client.list_tables is a Aws::DynamoDB::Types::ListTablesOutput struct with a table_names array.

I'm using aws-sdk-core (2.2.33) and aws-sdk-resources (2.2.33).
Any other thought?

Very interesting report. In my case, .value was nil and it was causing later lines of code doing a tables.include? to fail if I had no tables initially. Is there a reason why it would be bad to default to an empty array here? There's almost no performance cost and obviously there are edge cases where .value is nil. I leave it up to the community to decide. I'll keep using my fork if you want to get rid of that change since this works great for me.

Hey @adamthedeveloper what is the error that is returning without that @tables_.value || [] ? could be this one: Aws::DynamoDB::Errors::ResourceNotFoundException: Cannot do operations on a non-existent table?
If that's the case has to be with the caching that Dynamoid is doing of the tables names.

What's your version of aws-sdk-core and aws-sdk-resources in order if we can maybe narrow the issue and I'm happy to built a test case for it if we found the scenario

Do you have any insight @adamthedeveloper? just trying to find the scenario to properly wrap a test case for it.

Thanks!

sorry about being so slow to get back. I'll take a look tonight by bootstrapping a new project with the same dependencies and report back with the errors I saw originally. The error posted by @cavi21 doesn't look like the error I originally saw. The exception thrown was when .include? was being called on the tables method. I'd get .include? can't be called on nil.

Awesome and no worries! let me know If I can be useful for anything!

danielvidal · 2016-07-11T23:25:18Z

This PR is a little old and quite necessary. Don't you need some help to finish it?

adamthedeveloper · 2016-07-12T00:00:26Z

Thanks for the bump. I'll take a look tonight and report what my environment has in it. I'll write some tests too.

cavi21 · 2016-07-12T14:25:22Z

Also feel free te let me know if I can help on anything here @adamthedeveloper! thanks for the time

adamthedeveloper · 2016-07-12T15:36:49Z

Wasn't able to get to it last night. Will try again tonight.

adamthedeveloper · 2016-07-13T05:30:50Z

Here's the list of my Gems:

*** LOCAL GEMS ***

actioncable (5.0.0.beta2)
actionmailer (5.0.0.beta2)
actionpack (5.0.0.beta2)
actionview (5.0.0.beta2)
activejob (5.0.0.beta2)
activemodel (5.0.0.beta2)
activerecord (5.0.0.beta2)
activesupport (5.0.0.beta2)
arel (7.0.0)
aws-sdk (2.2.18)
aws-sdk-core (2.2.18)
aws-sdk-resources (2.2.18)
bigdecimal (1.2.8)
builder (3.2.2)
bundler (1.12.5)
bundler-unload (1.0.2)
byebug (8.2.2)
coderay (1.1.1)
concurrent-ruby (1.0.0)
did_you_mean (1.0.0)
dynamoid (0.7.1)
erubis (2.7.0)
executable-hooks (1.3.2)
gem-wrappers (1.2.7)
globalid (0.3.6)
i18n (0.7.0)
io-console (0.4.5)
jbuilder (2.4.1)
jmespath (1.1.3)
json (1.8.3)
loofah (2.0.3)
mail (2.6.3)
method_source (0.8.2)
mime-types (2.99)
mini_portile2 (2.0.0)
minitest (5.8.4, 5.8.3)
multi_json (1.11.2)
net-telnet (0.1.1)
nio4r (1.2.1)
nokogiri (1.6.7.2)
power_assert (0.2.6)
pry (0.10.3)
psych (2.0.17)
puma (2.16.0)
rack (2.0.0.alpha)
rack-test (0.6.3)
rails (5.0.0.beta2)
rails-deprecated_sanitizer (1.0.3)
rails-dom-testing (1.0.7)
rails-html-sanitizer (1.0.3)
railties (5.0.0.beta2)
rake (10.5.0, 10.4.2)
rdoc (4.2.1)
redis (3.2.2)
rubygems-bundler (1.4.4)
rvm (1.11.3.9)
slop (3.6.0)
spring (1.6.3)
sprockets (3.5.2)
sprockets-rails (3.0.1)
test-unit (3.1.5)
thor (0.19.1)
thread_safe (0.3.5)
tzinfo (1.2.2)
websocket-driver (0.6.3)
websocket-extensions (0.1.2)

Running a server fails instantly with:

/usr/local/rvm/gems/ruby-2.3.0@dynamotest2/gems/bundler-1.12.5/lib/bundler/runtime.rb:89:in rescue in block (2 levels) in require': There was an error while trying to load the gem 'dynamoid'. (Bundler::GemRequireError) Gem Load Error is: uninitialized constant ActiveModel::Serializers::Xml Backtrace for gem load error is: /usr/local/rvm/gems/ruby-2.3.0@dynamotest2/gems/dynamoid-0.7.1/lib/dynamoid/components.rb:26:inmodule:Components'
/usr/local/rvm/gems/ruby-2.3.0@dynamotest2/gems/dynamoid-0.7.1/lib/dynamoid/components.rb:6:in <module:Dynamoid>' /usr/local/rvm/gems/ruby-2.3.0@dynamotest2/gems/dynamoid-0.7.1/lib/dynamoid/components.rb:2:in<top (required)>'
/usr/local/rvm/gems/ruby-2.3.0@dynamotest2/gems/dynamoid-0.7.1/lib/dynamoid.rb:22:in require' /usr/local/rvm/gems/ruby-2.3.0@dynamotest2/gems/dynamoid-0.7.1/lib/dynamoid.rb:22:in<top (required)>'
/usr/local/rvm/gems/ruby-2.3.0@dynamotest2/gems/bundler-1.12.5/lib/bundler/runtime.rb:86:in require' /usr/local/rvm/gems/ruby-2.3.0@dynamotest2/gems/bundler-1.12.5/lib/bundler/runtime.rb:86:inblock (2 levels) in require'
/usr/local/rvm/gems/ruby-2.3.0@dynamotest2/gems/bundler-1.12.5/lib/bundler/runtime.rb:81:in each' /usr/local/rvm/gems/ruby-2.3.0@dynamotest2/gems/bundler-1.12.5/lib/bundler/runtime.rb:81:inblock in require'
/usr/local/rvm/gems/ruby-2.3.0@dynamotest2/gems/bundler-1.12.5/lib/bundler/runtime.rb:70:in each' /usr/local/rvm/gems/ruby-2.3.0@dynamotest2/gems/bundler-1.12.5/lib/bundler/runtime.rb:70:inrequire'
/usr/local/rvm/gems/ruby-2.3.0@dynamotest2/gems/bundler-1.12.5/lib/bundler.rb:102:in require' /Users/adammedeiros/Projects/rails/dynamotest/config/application.rb:17:in<top (required)>'
/usr/local/rvm/gems/ruby-2.3.0@dynamotest2/gems/railties-5.0.0.beta2/lib/rails/commands/commands_tasks.rb:88:in require' /usr/local/rvm/gems/ruby-2.3.0@dynamotest2/gems/railties-5.0.0.beta2/lib/rails/commands/commands_tasks.rb:88:inblock in server'
/usr/local/rvm/gems/ruby-2.3.0@dynamotest2/gems/railties-5.0.0.beta2/lib/rails/commands/commands_tasks.rb:85:in tap' /usr/local/rvm/gems/ruby-2.3.0@dynamotest2/gems/railties-5.0.0.beta2/lib/rails/commands/commands_tasks.rb:85:inserver'
/usr/local/rvm/gems/ruby-2.3.0@dynamotest2/gems/railties-5.0.0.beta2/lib/rails/commands/commands_tasks.rb:49:in run_command!' /usr/local/rvm/gems/ruby-2.3.0@dynamotest2/gems/railties-5.0.0.beta2/lib/rails/command.rb:20:inrun'
/usr/local/rvm/gems/ruby-2.3.0@dynamotest2/gems/railties-5.0.0.beta2/lib/rails/commands.rb:19:in <top (required)>' /Users/adammedeiros/Projects/rails/dynamotest/bin/rails:9:inrequire'
/Users/adammedeiros/Projects/rails/dynamotest/bin/rails:9:in <top (required)>' /usr/local/rvm/gems/ruby-2.3.0@dynamotest2/gems/spring-1.6.3/lib/spring/client/rails.rb:28:inload'
/usr/local/rvm/gems/ruby-2.3.0@dynamotest2/gems/spring-1.6.3/lib/spring/client/rails.rb:28:in call' /usr/local/rvm/gems/ruby-2.3.0@dynamotest2/gems/spring-1.6.3/lib/spring/client/command.rb:7:incall'
/usr/local/rvm/gems/ruby-2.3.0@dynamotest2/gems/spring-1.6.3/lib/spring/client.rb:28:in run' /usr/local/rvm/gems/ruby-2.3.0@dynamotest2/gems/spring-1.6.3/bin/spring:49:in<top (required)>'
/usr/local/rvm/gems/ruby-2.3.0@dynamotest2/gems/spring-1.6.3/lib/spring/binstub.rb:11:in load' /usr/local/rvm/gems/ruby-2.3.0@dynamotest2/gems/spring-1.6.3/lib/spring/binstub.rb:11:in<top (required)>'
/Users/adammedeiros/Projects/rails/dynamotest/bin/spring:13:in require' /Users/adammedeiros/Projects/rails/dynamotest/bin/spring:13:in<top (required)>'
bin/rails:3:in load' bin/rails:3:in

'
Bundler Error Backtrace:
from /usr/local/rvm/gems/ruby-2.3.0@dynamotest2/gems/bundler-1.12.5/lib/bundler/runtime.rb:85:in

block (2 levels) in require' from /usr/local/rvm/gems/ruby-2.3.0@dynamotest2/gems/bundler-1.12.5/lib/bundler/runtime.rb:81:in

each'
from /usr/local/rvm/gems/ruby-2.3.0@dynamotest2/gems/bundler-1.12.5/lib/bundler/runtime.rb:81:in block in require' from /usr/local/rvm/gems/ruby-2.3.0@dynamotest2/gems/bundler-1.12.5/lib/bundler/runtime.rb:70:ineach'
from /usr/local/rvm/gems/ruby-2.3.0@dynamotest2/gems/bundler-1.12.5/lib/bundler/runtime.rb:70:in require' from /usr/local/rvm/gems/ruby-2.3.0@dynamotest2/gems/bundler-1.12.5/lib/bundler.rb:102:inrequire'
from /Users/adammedeiros/Projects/rails/dynamotest/config/application.rb:17:in

<top (required)>' from /usr/local/rvm/gems/ruby-2.3.0@dynamotest2/gems/railties-5.0.0.beta2/lib/rails/commands/commands_tasks.rb:88:in

require'
from /usr/local/rvm/gems/ruby-2.3.0@dynamotest2/gems/railties-5.0.0.beta2/lib/rails/commands/commands_tasks.rb:88:in

block in server' from /usr/local/rvm/gems/ruby-2.3.0@dynamotest2/gems/railties-5.0.0.beta2/lib/rails/commands/commands_tasks.rb:85:in

tap'
from /usr/local/rvm/gems/ruby-2.3.0@dynamotest2/gems/railties-5.0.0.beta2/lib/rails/commands/commands_tasks.rb:85:in

server' from /usr/local/rvm/gems/ruby-2.3.0@dynamotest2/gems/railties-5.0.0.beta2/lib/rails/commands/commands_tasks.rb:49:in

run_command!'
from /usr/local/rvm/gems/ruby-2.3.0@dynamotest2/gems/railties-5.0.0.beta2/lib/rails/command.rb:20:in run' from /usr/local/rvm/gems/ruby-2.3.0@dynamotest2/gems/railties-5.0.0.beta2/lib/rails/commands.rb:19:in<top (required)>'
from /Users/adammedeiros/Projects/rails/dynamotest/bin/rails:9:in require' from /Users/adammedeiros/Projects/rails/dynamotest/bin/rails:9:in<top (required)>'
from /usr/local/rvm/gems/ruby-2.3.0@dynamotest2/gems/spring-1.6.3/lib/spring/client/rails.rb:28:in load' from /usr/local/rvm/gems/ruby-2.3.0@dynamotest2/gems/spring-1.6.3/lib/spring/client/rails.rb:28:incall'
from /usr/local/rvm/gems/ruby-2.3.0@dynamotest2/gems/spring-1.6.3/lib/spring/client/command.rb:7:in call' from /usr/local/rvm/gems/ruby-2.3.0@dynamotest2/gems/spring-1.6.3/lib/spring/client.rb:28:inrun'
from /usr/local/rvm/gems/ruby-2.3.0@dynamotest2/gems/spring-1.6.3/bin/spring:49:in <top (required)>' from /usr/local/rvm/gems/ruby-2.3.0@dynamotest2/gems/spring-1.6.3/lib/spring/binstub.rb:11:inload'
from /usr/local/rvm/gems/ruby-2.3.0@dynamotest2/gems/spring-1.6.3/lib/spring/binstub.rb:11:in <top (required)>' from /Users/adammedeiros/Projects/rails/dynamotest/bin/spring:13:inrequire'
from /Users/adammedeiros/Projects/rails/dynamotest/bin/spring:13:in <top (required)>' from bin/rails:3:inload'
from bin/rails:3:in `'

Changing the Gemfile to use gem 'dynamoid', '~> 1' and running bundle install causes:

`Bundler could not find compatible versions for gem "activemodel":
In Gemfile:
dynamoid (> 1) was resolved to 1.0.0, which depends on
activemodel (> 4)

rails (< 5.1, >= 5.0.0.beta2) was resolved to 5.0.0.beta2, which depends on
  activemodel (= 5.0.0.beta2)

rails (< 5.1, >= 5.0.0.beta2) was resolved to 5.0.0.beta2, which depends on
  activemodel (= 5.0.0.beta2)`

gwincr11 · 2016-07-13T22:49:53Z

@adamthedeveloper active model serializers xml is no long a part of Rails 5. You will need to include it as a dependency... Relates to #69

adamthedeveloper · 2016-07-13T22:56:09Z

I submit that it's not needed at all. Where in the code is it used? Can we
remove it as a dependency all
together? That's what I did in my fork and it works fine.

On Wed, Jul 13, 2016 at 3:49 PM, Zero Cool notifications@github.com wrote:

@adamthedeveloper https://github.com/adamthedeveloper active model
serializers xml is no long a part of Rails 5. You will need to include it
as a dependency... Relates to #69
#69

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#63 (comment), or mute
the thread
https://github.com/notifications/unsubscribe/AAJuqDce1YaGX6EeulPuXmFhsmyjZrA8ks5qVWuSgaJpZM4H7Xct
.

A.R. Medeiros

gwincr11 · 2016-07-13T22:58:35Z

I think you will need to bump up the version of active model to do that, but I am not certain. I am not really sure if it is used.

Also Rails 5 is now out, so you can rely on a version newer then Beta2 IMO.

adamthedeveloper · 2016-07-13T23:08:30Z

I have a unique environment edge case that required me to make some code
changes. I am posting the details of that environment as requested by
others. Yes - Rails 5 is out - does the non-beta version work better with
the current Dynamoid Gem version? Is that why you are calling out Rails 5
vs. Beta? Or are you making sure I know Rails 5 is out now?

Thanks

On Wed, Jul 13, 2016 at 3:58 PM, Zero Cool notifications@github.com wrote:

I think you will need to bump up the version of active model to do that,
but I am not certain. I am not really sure if it is used.

Also Rails 5 is now out, so you can rely on a version newer then Beta2 IMO.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#63 (comment), or mute
the thread
https://github.com/notifications/unsubscribe/AAJuqCttk9iOO9uyBs4xg0CKwgp-w5L6ks5qVW2bgaJpZM4H7Xct
.

A.R. Medeiros

gwincr11 · 2016-07-14T23:07:22Z

I am not sure if it works better, but some time dependencies change between versions so I was just pointing it out. @adamthedeveloper

pboling · 2016-09-04T20:38:45Z

Rails 5 support will be added in the next version to be released. Thanks for your work on this @adamthedeveloper & @gwincr11 . Will review soon.

pboling · 2016-09-08T15:47:53Z

@adamthedeveloper Please rebase on latest master - Spec suite is fixed!!!

bglusman · 2016-09-15T16:41:49Z

lib/dynamoid/components.rb

@@ -23,7 +23,6 @@ module Components
    include ActiveModel::Naming
    include ActiveModel::Observing if defined?(ActiveModel::Observing)
    include ActiveModel::Serializers::JSON
-    include ActiveModel::Serializers::Xml


commented similarly on another PR also adding rails 5 support, but don't know which will get fixed up first, so adding here too... beleive this should be include ActiveModel::Serializers::Xml if defined?(ActiveModel::Serializers::Xml) so people relying on it in old rails versions/adding it back manually via extracted gem can still use it.

adamthedeveloper · 2016-09-15T16:52:43Z

Hey @bglusman,

I actually question XML's use in past versions. I don't think it's used or
has ever been used. I think whoever added that line originally should
comment on why it was added and if it is necessary at all in any version.
To the original author of the gem, why did you need this serializer in the
first place? Is it needed? Was it ever needed at all? Did we start with it
and forget to do some cleanup or something?

On Thu, Sep 15, 2016 at 9:41 AM, Brian Glusman notifications@github.com
wrote:

@bglusman commented on this pull request.

In lib/dynamoid/components.rb
#63 (review):

@@ -23,7 +23,6 @@ module Components
include ActiveModel::Naming
include ActiveModel::Observing if defined?(ActiveModel::Observing)
include ActiveModel::Serializers::JSON

include ActiveModel::Serializers::Xml

commented similarly on another PR also adding rails 5 support, but don't
know which will get fixed up first, so adding here too... beleive this
should be include ActiveModel::Serializers::Xml if defined?(ActiveModel::
Serializers::Xml) so people relying on it in old rails versions/adding it
back manually via extracted gem can still use it.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#63 (review),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAJuqILukLGdSI18ivacACBQii5sL_w6ks5qqXVPgaJpZM4H7Xct
.

A.R. Medeiros

bglusman · 2016-12-29T03:30:42Z

Resolved by #109

adamthedeveloper added 7 commits February 20, 2016 01:00

Update dynamoid.gemspec

7bef2cf

allow this gem to run on rails 5

Update dynamoid.gemspec

a287588

adding the active_model_serializers runtime dependency

Update dynamoid.gemspec

27a84d0

Update components.rb

52a7e16

Update adapter.rb

19cf5df

Remove the xml dependency

f8428f4

In an effort to eliminate the XML errors I was seeing while running this in rails 5, I added active-model-serializers-xml as a runtime dependency. It changed nothing, I was still getting those errors.

Remove the inclusion of the XML Serializer

efe9e6c

This was throwing errors stating that ActiveModel::Serializers::XML couldn't be found. After removing it, everything seemed to work fine for me. What is it being used for?

adamthedeveloper mentioned this pull request Mar 30, 2016

undefined method `include?' for nil:NilClass #44

Closed

cavi21 reviewed Apr 10, 2016
View reviewed changes

cavi21 mentioned this pull request Apr 20, 2016

Rails 5 #43

Closed

bglusman reviewed Sep 15, 2016

View reviewed changes

bglusman closed this Dec 29, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make Dynamoid Rails 5 compatible and help avoid errors when no tables exist yet #63

Make Dynamoid Rails 5 compatible and help avoid errors when no tables exist yet #63

adamthedeveloper commented Mar 30, 2016

coveralls commented Mar 30, 2016

pcorpet commented Mar 30, 2016

adamthedeveloper commented Mar 30, 2016

cavi21 Apr 10, 2016

adamthedeveloper Apr 11, 2016

cavi21 Apr 11, 2016

cavi21 Apr 20, 2016

adamthedeveloper Apr 20, 2016

cavi21 Apr 20, 2016

danielvidal commented Jul 11, 2016

adamthedeveloper commented Jul 12, 2016

cavi21 commented Jul 12, 2016

adamthedeveloper commented Jul 12, 2016

adamthedeveloper commented Jul 13, 2016

gwincr11 commented Jul 13, 2016

adamthedeveloper commented Jul 13, 2016

gwincr11 commented Jul 13, 2016

adamthedeveloper commented Jul 13, 2016

gwincr11 commented Jul 14, 2016

pboling commented Sep 4, 2016 •

edited

Loading

pboling commented Sep 8, 2016 •

edited

Loading

bglusman Sep 15, 2016

adamthedeveloper commented Sep 15, 2016

@bglusman commented on this pull request.

bglusman commented Dec 29, 2016

Make Dynamoid Rails 5 compatible and help avoid errors when no tables exist yet #63

Make Dynamoid Rails 5 compatible and help avoid errors when no tables exist yet #63

Conversation

adamthedeveloper commented Mar 30, 2016

coveralls commented Mar 30, 2016

pcorpet commented Mar 30, 2016

adamthedeveloper commented Mar 30, 2016

cavi21 Apr 10, 2016

Choose a reason for hiding this comment

adamthedeveloper Apr 11, 2016

Choose a reason for hiding this comment

cavi21 Apr 11, 2016

Choose a reason for hiding this comment

cavi21 Apr 20, 2016

Choose a reason for hiding this comment

adamthedeveloper Apr 20, 2016

Choose a reason for hiding this comment

cavi21 Apr 20, 2016

Choose a reason for hiding this comment

danielvidal commented Jul 11, 2016

adamthedeveloper commented Jul 12, 2016

cavi21 commented Jul 12, 2016

adamthedeveloper commented Jul 12, 2016

adamthedeveloper commented Jul 13, 2016

gwincr11 commented Jul 13, 2016

adamthedeveloper commented Jul 13, 2016

gwincr11 commented Jul 13, 2016

adamthedeveloper commented Jul 13, 2016

gwincr11 commented Jul 14, 2016

pboling commented Sep 4, 2016 • edited Loading

pboling commented Sep 8, 2016 • edited Loading

bglusman Sep 15, 2016

Choose a reason for hiding this comment

adamthedeveloper commented Sep 15, 2016

@bglusman commented on this pull request.

bglusman commented Dec 29, 2016

pboling commented Sep 4, 2016 •

edited

Loading

pboling commented Sep 8, 2016 •

edited

Loading