dflook / cloudformation-dns-certificate

Cloudformation DNS Validated Certificate Resource
MIT License
48 stars 13 forks source link

Throttling exception when a large number of certificates exists #12

Closed danieljamesscott closed 3 years ago

danieljamesscott commented 3 years ago

When a large number of certificates exists, this requests to fetch the certificate's tags:

https://github.com/dflook/cloudformation-dns-certificate/blob/master/src/troposphere_dns_certificate/certificate.py#L124

causes a throttling exception. I think that a nice solution would be to check if p['DomainName'] == certificate['DomainName'] (Not sure if we need case sensitivity and/or a trailing .) before attempting to fetch the tags. Is this a good idea?

dflook commented 3 years ago

Hi @danieljamesscott, that seems sensible. How many certificates do you have? Do you have a traceback of the exception?

danieljamesscott commented 3 years ago

I have 40 or so certificates, but I am also trying to update 9 certificates in 1 stack, so they all run in parallel.

I can see 14 or so log lines for the certificate object, and then I get


  File "/var/task/index.py", line 87, in handler
    try:D=J(A7,region_name=e[AB].get(m));C(A9);V(W(e[AB]))
  File "/var/task/index.py", line 39, in W
    C(A);B={B[P]:B[G]for B in D.list_tags_for_certificate(**{F:A[F]})[E]}
  File "/var/runtime/botocore/client.py", line 357, in _api_call
    return self._make_api_call(operation_name, kwargs)
  File "/var/runtime/botocore/client.py", line 676, in _make_api_call
    raise error_class(parsed_response, operation_name)
botocore.exceptions.ClientError: An error occurred (ThrottlingException) when calling the ListTagsForCertificate operation (reached max retries: 4): Rate exceeded```
danieljamesscott commented 3 years ago

I have tested the change, and it seems to work fine. I had to remove a couple of log lines, to avoid the 4096k code size limit. Should I do a PR for this?

dflook commented 3 years ago

Thanks @danieljamesscott, that would be great.

danieljamesscott commented 3 years ago

Done https://github.com/dflook/cloudformation-dns-certificate/pull/13

I added a test to check that the code was below 4096 bytes. I just installed pytest and ran it using that.

danieljamesscott commented 3 years ago

Sorry to bug you, but when do you think you'll be able to merge/release this?

dflook commented 3 years ago

No problem, I'm hoping to have time in the next couple of days.

dflook commented 3 years ago

Thanks for your PR, it's now merged and released in 1.7.3 🎉

danieljamesscott commented 3 years ago

Excellent. Thanks!