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

Prototype update for SNS tests in #334 #342

Conversation

smarnach
Copy link
Contributor

@smarnach smarnach commented Sep 16, 2020

This prototype encapsulates extracting region and profile in a new method on the BotocoreClient to allow the client code to look reasonable. It's not quite what I had in mind, but with the current structure it's the best I could come up with in an hour.

I also factored out the results list into its own class. It was a bit weird that the client stored the results in itself and then modified itself with extract_key() and flatten(). I simply moved that to a separate class, so subsequent calls to the client don't overwrite the previous results.

The test is now working and passing, but it's terribly slow. I tried it for the dev account, and it makes some 3,000 API calls, so you get rate limited and the client waits and retries. When I limit it to 300 subscriptions, it returns quite quickly, but running it for all subscriptions takes ages.

If the general approach seems reasonable, at least as an incremental improvement over the current situation, I can factor out the changes in the client in a separate PR and clean them up a bit.

Related: #334

@Micheletto Micheletto mentioned this pull request Sep 16, 2020
@smarnach
Copy link
Contributor Author

I'll update this PR once #334 is merged – probably next week.

@g-k
Copy link
Contributor

g-k commented Sep 17, 2020

👍 on landing the client changes

I also factored out the results list into its own class. It was a bit weird that the client stored the results in itself and then modified itself with extract_key() and flatten(). I simply moved that to a separate class, so subsequent calls to the client don't overwrite the previous results.

That'd be great! I used to write clojure and wanted something like https://clojuredocs.org/clojure.core/-%3E or a fluent interface, but it turned out badly (strange and incomplete). refs: #11

I pulled out a separate frost CLI entrypoint recently in #341 and adding a new subcommand e.g. frost fetch might provide the flexibility to try a saner client (e.g. boto3) and caching (refs #117) without having to deal with pytest.

@smarnach
Copy link
Contributor Author

I'll clean up the client changes and file them as a new PR against master.

@smarnach smarnach closed this Sep 22, 2020
@smarnach
Copy link
Contributor Author

Draft PR opened: #356

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