Skip to content

Add collect_and_retry#7

Merged
softprops merged 1 commit intosoftprops:developfrom
endor:collect_and_retry
Jul 7, 2020
Merged

Add collect_and_retry#7
softprops merged 1 commit intosoftprops:developfrom
endor:collect_and_retry

Conversation

@endor
Copy link
Copy Markdown
Contributor

@endor endor commented Jul 4, 2020

I am not entirely happy with making S :Clone, but I couldn't figure out a better way to make this work.

@softprops
Copy link
Copy Markdown
Owner

Does this obviate the motivation to use collect. Really want to aim for an api with a small surface area where each area has common usecases. If collect was not helpful and motivated another form of collect, is there a way to collapse these into one in a way that is maximally useful.

I feel like I need to stew on this a bit. There's definitely something here but Im just not sure what the right interface is for it yet

@endor
Copy link
Copy Markdown
Contributor Author

endor commented Jul 4, 2020

I suppose for our use case, we'll probably go with collect_and_retry, since there will be the potential for errors we want to catch. Or to put it more concretely, while we might have unprocessed keys in the Dynamo success response, we might get provisioned throughput exceeded errors when trying to fetch those unprocessed keys and would like to retry again. So we could remove the collect and just keep the collect_and_retry, if a small surface area is a goal.

@endor
Copy link
Copy Markdown
Contributor Author

endor commented Jul 7, 2020

I'll be on vacation for the next couple of weeks. I can fix any issues that might arise after that.

@softprops
Copy link
Copy Markdown
Owner

I'll merge this but might make some changes after. Check in when you get back. Enjoy your vacation!

@softprops softprops merged commit deff4b8 into softprops:develop Jul 7, 2020
@endor
Copy link
Copy Markdown
Contributor Author

endor commented Aug 31, 2020

So.. where should we go with this? Can a new version be released or should we still do some changes?

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants