Skip to content

Conversation

@joe-irving
Copy link
Contributor

Added functionality for:

I have added to the Readme and added tests - and would like to work through the rest of the API at some point but needed these for a project. Core things that are missing that might be an issue are:

  • Submissions for forms
  • Outreaches for Advocacy Campaigns

Is it okay to merge?

With relevent bits in the docs and tests
with testing & example in docs
Copy link
Contributor

@anero anero left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for submitting this PR Joe! I think it looks mostly right, apart from the examples on README I think it would be good to handle hitting the rate limit on calls to all.
Docs are unclear on the response when hitting the rate limit but I imagine we'll get an exception from ActionNetworkRest::Response::RaiseError if Action Network returns a status code with error.

objects
end

def all
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like you can easily go over the rate limit while using this method. May be worth adding some logic for retying with a slight delay if we reach it

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added to this method so it now limits the rate is calls list to every 0.25 secs max (as the docs mention). It also exponentially backs off if the 429 (Too Many Requests) response code is received. I think this should do it.

Worth noting this makes running bundle exec rake -t a little slower because it test the back off is working okay.

And minor readme changes
Consistent timestamp as real number

Co-authored-by: Diego Marcet <[email protected]>
@anero anero merged commit f8807cf into controlshift:main Sep 26, 2022
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