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

[FIX] require aws-sdk-core before requiring the aws related libraries #140

Merged
merged 1 commit into from
Mar 9, 2018
Merged

[FIX] require aws-sdk-core before requiring the aws related libraries #140

merged 1 commit into from
Mar 9, 2018

Conversation

kenju
Copy link

@kenju kenju commented Feb 9, 2018

introduced at https://github.com/awslabs/aws-fluent-plugin-kinesis/pull/131/files

Why

aws-sdk-core library should be installed in the first place to get the version by Gem.loaded_specs['aws-sdk-core'].

However in the following logic, aws-sdk-core is not required and Gem.loaded_specs['aws-sdk-core'] returns nil. Then NoMethoError would be thrown.

      def aws_sdk_v2?
        # Gem.loaded_specs['aws-sdk-core'] #=> nil
        @aws_sdk_v2 ||= Gem.loaded_specs['aws-sdk-core'].version < Gem::Version.create('3')
      end
      ...
          if aws_sdk_v2?
            # these library require "aws-sdk-core" as a dependency library
            # Therefore, before requiring "aws-sdk" or "aws-sdk-kinesis", 
            # Gem.loaded_specs['aws-sdk-core'] returns nil
            require 'aws-sdk'
          else
            require 'aws-sdk-kinesis'
          end

Solution

I simply put require 'aws-sdk-core' in the first line of the client.rb. How do you think about this?

How to reproduce

simply run ruby test/helper.rb on the local environmnet.

 aws-fluent-plugin-kinesis (master)    ruby test/helper.rb
Traceback (most recent call last):
	1: from test/helper.rb:39:in `<main>'
test/helper.rb:21:in `aws_sdk_v2?': undefined method `version' for nil:NilClass (NoMethodError)

@riywo
Copy link
Contributor

riywo commented Feb 13, 2018

Hi @kenju ,

Thank you for reporting the issue. We can merge this patch, but before doing that, we'd like to understand why you want to run the script in a pure ruby environment instead of bundle exec. Could you explain more detail of your use case?

Best,

@kenju
Copy link
Author

kenju commented Feb 13, 2018

Hi @riywo

Thank you for reporting the issue. We can merge this patch, but before doing that, we'd like to understand why you want to run the script in a pure ruby environment instead of bundle exec

Yes, sure. We found this behavior while updating this library onto the fluentd. The fluentd agent is running on the ECS environment (a Docker container), where the bundler is not installed.

(I think this is difficult to find via test, because the test environment is running with bundle exec, so that the bundler automatically resolve the dependencies)

Does it make sense for you?

Thanks!

@fujiwara
Copy link

fujiwara commented Mar 9, 2018

@riywo
We reproduced this issue on my environment too.

  • Amazon ECS
  • base image fluent/fluentd:v1.1.0-debian-onbuild
  • fluent-plugin-kinesis 2.1.0

@riywo riywo merged commit cc0eaf8 into awslabs:master Mar 9, 2018
@riywo
Copy link
Contributor

riywo commented Mar 9, 2018

Thank you for reporting. I'll release this patch with v2.1.1 soon.

@riywo
Copy link
Contributor

riywo commented Mar 9, 2018

@kenju kenju deleted the fix-require-dependencies branch March 11, 2018 02:46
@kenju
Copy link
Author

kenju commented Mar 11, 2018

Thanks for releasing!

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

Successfully merging this pull request may close these issues.

3 participants