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

[Utils] Dynamic retryable function wrapper #902

Closed
wants to merge 4 commits into from

Conversation

timgrein
Copy link
Contributor

@timgrein timgrein commented May 11, 2023

Closes https://github.com/elastic/enterprise-search-team/issues/4525

With the current @retryable decorator we're not able to set retries, retry_interval at runtime using fields as decorators are not dynamically updated at runtime. Therefore we need a wrapper function, which supports this behavior to support dynamic advanced configuration values for retrying.

Checklists

Pre-Review Checklist

  • this PR has a meaningful title
  • this PR links to all relevant github issues that it fixes or partially addresses
  • if there is no GH issue, please create it. Each PR should have a link to an issue
  • this PR has a thorough description
  • Covered the changes with automated tests
  • Tested the changes locally
  • Added a label for each target release version (example: v7.13.2, v7.14.0, v8.0.0)
    - [ ] Considered corresponding documentation changes
    - [ ] Contributed any configuration settings changes to the configuration reference

@tarekziade
Copy link
Contributor

why two distinct decorators? you can merge both and have both mechanism from with one single smart decorator

@timgrein
Copy link
Contributor Author

timgrein commented May 12, 2023

why two distinct decorators? you can merge both and have both mechanism from with one single smart decorator

What do you mean by smart decorator? Is there an option in python to have a decorator, which supports something like this:

@retryable(retries=self.retries, ...)
def some_method()

Specifically referencing a field of an object, which can be updated during runtime. This PR adds a function wrapper, which is not a decorator AFAIU to support the behavior I've described above.

@tarekziade
Copy link
Contributor

why two distinct decorators? you can merge both and have both mechanism from with one single smart decorator

What do you mean by smart decorator? Is there an option in python to have a decorator, which supports something like this:

@retryable(retries=self.retries, ...)
def some_method()

yeah the retryable function can have this signature :

def retryable(*args, **kw):
   if callable(args[0]):
       func =  args[0]
       etc..
   else:
      etc..

So you don't have two different decorators, it stays an internal implementation detail

@artem-shelkovnikov
Copy link
Member

@timgrein should we close this PR or follow-up?

@timgrein timgrein closed this Feb 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants