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

add rate limit handling #25327

Conversation

jthom-vmray
Copy link
Contributor

@jthom-vmray jthom-vmray commented Mar 16, 2023

Contributing to Cortex XSOAR Content

Make sure to register your contribution by filling the contribution registration form

The Pull Request will be reviewed only after the contribution registration form is filled.

Status

  • In Progress
  • Ready
  • In Hold - (Reason for hold)

Related Issues

fixes: link to the issue

Description

The app could not handle rate limiting, which lead to failing playbook runs. This PR introduces a new setting that handles rate limiting, if set.

Screenshots

Paste here any images that will help the reviewer

Minimum version of Cortex XSOAR

  • 6.0.0
  • 6.1.0
  • 6.2.0
  • 6.5.0

Does it break backward compatibility?

  • Yes
    • Further details:
  • No

Must have

  • Tests
  • Documentation

@content-bot content-bot added the Contribution Thank you! Contributions are always welcome! label Mar 16, 2023
@content-bot content-bot changed the base branch from master to contrib/jthom-vmray_add-rate-limit-handling March 16, 2023 13:22
@content-bot
Copy link
Collaborator

Thank you for your contribution. Your generosity and caring are unrivaled! Make sure to register your contribution by filling the Contribution Registration form, so our content wizard @daryakoval will know the proposed changes are ready to bereviewed.

@content-bot content-bot added Contribution Form Filled Whether contribution form filled or not. Partner labels Mar 16, 2023
@gal-forer gal-forer removed the request for review from daryakoval March 17, 2023 00:16
@gal-forer gal-forer assigned dansterenson and unassigned daryakoval Mar 17, 2023
Copy link
Contributor

@dansterenson dansterenson left a comment

Choose a reason for hiding this comment

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

Nice and thanks for your contribution. Please see my notes.

demisto.debug(f"Rate limit exceeded! Waiting {seconds_to_wait} seconds and then retry.")
time.sleep(seconds_to_wait) # pylint: disable=sleep-exists
reset_file_buffer(files)
return http_request(method, url_suffix, params, files, ignore_errors, get_raw)
Copy link
Contributor

Choose a reason for hiding this comment

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

In this case, it would be better to assign the response to a variable and then continue with the logic of the function.

Suggested change
return http_request(method, url_suffix, params, files, ignore_errors, get_raw)
r = http_request(...)

Why are the request args different from the one above?

Suggested change
return http_request(method, url_suffix, params, files, ignore_errors, get_raw)
r = requests.request(
method, url, params=params, headers=HEADERS, files=files, verify=USE_SSL, proxies=PROXIES
)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The arguments differ, because it is a recursive function call and not a direct request. We need to call http_request recursively here, since it is possible that retrying only once could end up in a rate limit that was triggered by another waiting request.

Copy link
Contributor

Choose a reason for hiding this comment

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

Missed the recursion. So what is the stop condition? How do you make sure you won't enter an infinite loop and get a stack overflow? you should count the number of tries.

Packs/VMRay/Integrations/VMRay/README.md Outdated Show resolved Hide resolved
Packs/VMRay/Integrations/VMRay/VMRay.py Outdated Show resolved Hide resolved
Packs/VMRay/Integrations/VMRay/VMRay.py Outdated Show resolved Hide resolved
Packs/VMRay/Integrations/VMRay/VMRay_test.py Outdated Show resolved Hide resolved
Packs/VMRay/ReleaseNotes/1_1_7.md Outdated Show resolved Hide resolved
jthom-vmray and others added 5 commits March 20, 2023 14:04
Co-authored-by: Dan Sterenson <38375556+dansterenson@users.noreply.github.com>
Co-authored-by: Dan Sterenson <38375556+dansterenson@users.noreply.github.com>
Co-authored-by: Dan Sterenson <38375556+dansterenson@users.noreply.github.com>
Co-authored-by: Dan Sterenson <38375556+dansterenson@users.noreply.github.com>
demisto.debug(f"Rate limit exceeded! Waiting {seconds_to_wait} seconds and then retry.")
time.sleep(seconds_to_wait) # pylint: disable=sleep-exists
reset_file_buffer(files)
return http_request(method, url_suffix, params, files, ignore_errors, get_raw)
Copy link
Contributor

Choose a reason for hiding this comment

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

Missed the recursion. So what is the stop condition? How do you make sure you won't enter an infinite loop and get a stack overflow? you should count the number of tries.

Copy link
Contributor

@dansterenson dansterenson left a comment

Choose a reason for hiding this comment

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

Nice

@dansterenson dansterenson merged commit 41d53ec into demisto:contrib/jthom-vmray_add-rate-limit-handling Mar 22, 2023
10 of 12 checks passed
@content-bot content-bot mentioned this pull request Mar 22, 2023
11 tasks
dansterenson added a commit that referenced this pull request Mar 22, 2023
* add rate limit handling

* Update Packs/VMRay/Integrations/VMRay/README.md



* Update Packs/VMRay/Integrations/VMRay/VMRay.py



* Update Packs/VMRay/Integrations/VMRay/VMRay.py



* Update Packs/VMRay/ReleaseNotes/1_1_7.md



* use 'call_count' to verify rate limit triggered

* add counter for rate limit reties

* add unit test for max reties

---------

Co-authored-by: Jens Thom <72789379+jthom-vmray@users.noreply.github.com>
Co-authored-by: Dan Sterenson <38375556+dansterenson@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Contribution Form Filled Whether contribution form filled or not. Contribution Thank you! Contributions are always welcome! Partner
Projects
None yet
5 participants