Allow BroadwaySQS to work with temporary sessionToken

[msaraiva]

Discussion started here elixir-broadway/broadway#68.

@gvaughn:

Will I have to write a custom Producer to make this possible? Maybe with just a custom SqsClient?

Currently, the only option would be to create a custom SQSClient. However, we believe it would be a good idea to extend the API to allow this, so here's is a first suggestion:

We can provide a new config option for the client called token_generator, which is a {M, f, a} to be called before each receive/delete request. An initial implementation of this generator could be a GenServer that periodically updates the token (maybe on an :ets table with read_concurrency: true?).

In case you see other scenarios where we might need to update other config options, we could even consider expanding this concept to a more generic one like extra_options_generator that could be used to merge any other option to the config.

Thoughts?

[gvaughn]

Thanks for your thoughts on this issue.

I'm still new to this codebase, so take my comments with that in mind. What would this token_generator be expected to return? Is it the:

config: [
               access_key_id: "abc",
               secret_access_key: "def",
               security_token: "ghi"
             ]

information?

if that is the case, I'd prefer (and is likely more general) to receive the old config as a sort of state or accumulator which it can modify or return unchanged. For this to work, I'd also need to be able to store some extra keys -- at least the expiration time. This approach would still allow a token_generator to use ETS or. GenServer, but would not require it to.

[josevalim]

if that is the case, I'd prefer (and is likely more general) to receive the old config as a sort of state or accumulator which it can modify or return unchanged. For this to work, I'd also need to be able to store some extra keys -- at least the expiration time.

The problem with this approach is that you may have multiple producers... so we would need to call the callback with each of config of each producer, which will complicate things. For example, the expiration time would be per producer, so if you want to rely on it, then it means caching between producers won't work.

Also, even if you have one producer, the AWS client can be called from many different processes. If you have to pass the config, you transform a read bottleneck in a write bottleneck, and that would slow everything down.

So there is a bunch of complexity in making the solution more general because of the nature of the domain. Therefore I would prefer to allow only to return from token_generator (or extra_config_generator) without giving you the current config unless you have a strong argument to receive the current config.

[gvaughn]

I see. I was too focused on the problem in front of me which only has a single producer (at least for now ;-) and I'm still getting familiar with Broadway. I don't fully see all the subtleties yet, but I trust your judgement. I can achieve what I'm looking for by managing my own state behind my *_generator

[gvaughn]

I now have a working implementation of my custom SQSClient and can better comment on this issue. Yes a {M, f, a} token_generator called before each read/delete request would serve my needs. I have to store my state in one spot, so I'll have this bottleneck (which isn't a problem at our rate of messages) but we would not want to bake that into the framework itself.

On the other hand, I also wouldn't mind if the answer to this need is to write a custom SQSClient. It wasn't as daunting as it seemed when I was first learning Broadway. I've used mine to provide some optional parameters that let me simulate more situations in tests. It might be nice if some private functions in the ExAwsCllient were made public, but it's not so complex that it bothers me to re-implement it either.

[josevalim]

This functionality was merged for Google Cloud PubSub, so we would definitely be glad to merge similar PR here: elixir-broadway/broadway_cloud_pub_sub#29

So a PR is definitely welcome!