Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

jets seems to be incompatible with Dynomite #65

Closed
graimon opened this issue Nov 8, 2018 · 3 comments
Closed

jets seems to be incompatible with Dynomite #65

graimon opened this issue Nov 8, 2018 · 3 comments

Comments

@graimon
Copy link

graimon commented Nov 8, 2018

Hello, first of all I want to congratulate you for the wonderful framework you've built. Jets is amazing!

I was playing around with it, running some tests and came with a really strange behaviour.
I was trying to publish a notification to an SNS queue, following the docs and examples and everything went smooth.
Then I decided to add some logic persisting data on a DynamoDB table, using Dynomite and everything started failing.

After digging and running several tests I came to the conclusion that the problem has to do with the following line on dynomite or something related to Aws.config.

I'll try to describe the failing situation:

  • I'm using DYNAMODB_ENDPOINT=https://dynamodb.eu-west-1.amazonaws.com in my .env files
  • I have a Dynomite::Item:
class Channel < ApplicationItem
  partition_key "name"
end
  • I have an Alert.rb similar to the docs one:
class Alert < Jets::Stack
  sns_topic(:delivery_completed, display_name: "cool topic")
end
  • And then a simple controller:
class ChannelsController < ApplicationController
  include Jets::AwsServices
  class_iam_policy("dynamodb", "sns")
  def index
    channels = Channel.scan
    topic_arn = Alert.lookup(:delivery_completed)
    Jets.logger.debug("About to publish to #{topic_arn}")
    sns.publish(topic_arn: topic_arn,  subject: "Test", message: "Just testing")
    render json: channels.to_json
  end
end

Using this scenario I am able to get the channels, but when I try to lookup for the arn I get the following exception:

{
  "stackTrace": [
    "/tmp/bundled/gems/ruby/2.5.0/gems/aws-sdk-core-3.36.0/lib/seahorse/client/plugins/raise_response_errors.rb:15:in `call'",
    "/tmp/bundled/gems/ruby/2.5.0/gems/aws-sdk-core-3.36.0/lib/aws-sdk-core/plugins/jsonvalue_converter.rb:20:in `call'",
    "/tmp/bundled/gems/ruby/2.5.0/gems/aws-sdk-core-3.36.0/lib/aws-sdk-core/plugins/idempotency_token.rb:17:in `call'",
    "/tmp/bundled/gems/ruby/2.5.0/gems/aws-sdk-core-3.36.0/lib/aws-sdk-core/plugins/param_converter.rb:24:in `call'",
    "/tmp/bundled/gems/ruby/2.5.0/gems/aws-sdk-core-3.36.0/lib/aws-sdk-core/plugins/response_paging.rb:10:in `call'",
    "/tmp/bundled/gems/ruby/2.5.0/gems/aws-sdk-core-3.36.0/lib/seahorse/client/plugins/response_target.rb:23:in `call'",
    "/tmp/bundled/gems/ruby/2.5.0/gems/aws-sdk-core-3.36.0/lib/seahorse/client/request.rb:70:in `send_request'",
    "/tmp/bundled/gems/ruby/2.5.0/gems/aws-sdk-sns-1.7.0/lib/aws-sdk-sns/client.rb:1204:in `publish'",
    "/var/task/app/controllers/channels_controller.rb:9:in `inde [TRUNCATED]"
  ]
}

The catchy thing is that if I lookup the arn first and then scan the dynamo model, the first time the function is invoked, it works as expected, the consequent calls fail.

class ChannelsController < ApplicationController
  include Jets::AwsServices
  class_iam_policy("dynamodb", "sns")
  def index
    topic_arn = Alert.lookup(:delivery_completed)
    Jets.logger.debug("About to publish to #{topic_arn}")
    sns.publish(topic_arn: topic_arn,  subject: "Test", message: "Just testing")
    channels = Channel.scan
    render json: channels.to_json
  end
end

I hope the description is clear enough.

RaY

@tongueroo
Copy link
Collaborator

@graimon Thanks for the nice words.

Interesting. Wasn't able to reproduce the issue. 😕

Here's the code based on your examples: https://github.com/tongueroo/dynamodb-demo

Ran the channels#index multiple times without fail 🧐

lambda-function-channels-index

Manually subscribe to the SNS topic and got the emails:

email-sns-test

Am unable to reproduce right now. Doesn't mean there's no issue, just am unable to reproduce on my setup. Maybe the full CloudWatch logs will reveal a more debugging info what you're seeing? Here's a post on how to view the CloudWatch logs: Jets Tutorial Debugging Logs Part 3: AWS Lambda Ruby

Let me know. Leaving open for now in case there's more info.

P.S. Impressed that you were able to use Dynomite, would like to improve the docs on it, too busy with work though 😁

@tongueroo tongueroo changed the title jets seems to be incompatible with Dynamite jets seems to be incompatible with Dynomite Nov 9, 2018
@graimon
Copy link
Author

graimon commented Nov 12, 2018

Hi again,

Looking at the differences with your project, the only thing I can see is that I'm using the DYNAMODB_ENDPOINT env variable, which now I realise it is not needed unless you want to access some other region.

Looking at Dynomite::DbConfig, it is updating the Aws.config.endpoint, this is why maybe I'm getting this strange behaviour.

def db
      return @@db if @@db

      config = db_config
      endpoint = ENV['DYNAMODB_ENDPOINT'] || config['endpoint']
      check_dynamodb_local!(endpoint)

      Aws.config.update(endpoint: endpoint) if endpoint
      @@db ||= Aws::DynamoDB::Client.new
    end

It may be worth changing the way the endpoint is passed to the Aws::DynamoDB::Client.new on Dynomite itself, what do you think?

But thanks! I've removed the env variable on my project and everything started working as expected.

Cheers,

@tongueroo
Copy link
Collaborator

@graimon Glad that it worked when not setting DYNAMODB_ENDPOINT. 👍

RE: It may be worth changing the way the endpoint is passed to the Aws::DynamoDB::Client.new on Dynomite itself, what do you think?

Good point! The endpoint setting should be scoped to the DynamoDB client only. Can see how setting it globally like that can cause issues. Updated dynomite and released in v1.0.8. Dynomite Changelog Updated the vendor/dynomite submodule reference in jets project master branch and this will go out in the next Jets release.

Thanks again for the report. Closing this out now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants