amazon-archives / aws-flow-ruby

ARCHIVED
137 stars 58 forks source link

Flow Framework doesn't allow per-service credentials? #22

Closed Jud closed 10 years ago

Jud commented 10 years ago

I mistakenly thought that 13f689c69735347374b7f078aa1d0862d1792cc6 (#20) had broken the ability to provider per-service credentials, but after stepping through different versions of the gem, I realized that this has never worked:

swf = AWS::SimpleWorkflow.new(access_key_id: AWS_SWF_KEY, secret_access_key: AWS_SWF_SECRET)
domain = swf.domains[SWF_DOMAIN]

activity_worker = AWS::Flow::ActivityWorker.new(swf.client, domain, task_list, ClassName)
activity_worker.start if __FILE__ == $0

When a new task is received and the process forks, the child's response fails due to missing credentials.

I haven't checked over the code too closely, but is there a reason that @service needs to be defined again here: https://github.com/aws/aws-flow-ruby/blob/13f689c69735347374b7f078aa1d0862d1792cc6/aws-flow/lib/aws/decider/task_poller.rb#L136 ?

Looks like it is set in initialize.

mjsteger commented 10 years ago

That line is there to ensure there aren't any problems with the http pool and any shared objects in them; before we added that line, we found that we were getting timeouts very frequently for forked activities. It's possible that it's not necessary, though. Assuming it's necessary, it looks like taking the old service's config via something like:

previous_config = @service.config.to_h
previous_config.delete(:http_handler)
@service = AWS::SimpleWorkflow.new.client
@service = @service.with_http_handler(AWS::Core::Http::NetHttpHandler.new(previous_config))

would be the best way to go. Does that make sense? I'll also add a test case to make sure that it's possible to run activity/workflow workers with a different configuration than AWS.config

Jud commented 10 years ago

@mjsteger I tried taking that line out and ran into the timeout issues. Any ideas as to why they occur? Seems like the real fix is finding out the cause of the timeouts, but the above will work for now.