Skip to content

Commit

Permalink
chore: remove retry dependency in favor of backoff (apache#15788)
Browse files Browse the repository at this point in the history
* chore: remove retry dep in favor of backoff

* Fix lint
  • Loading branch information
betodealmeida authored Jul 20, 2021
1 parent cbd3780 commit c9dad05
Show file tree
Hide file tree
Showing 7 changed files with 53 additions and 13 deletions.
2 changes: 1 addition & 1 deletion setup.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ combine_as_imports = true
include_trailing_comma = true
line_length = 88
known_first_party = superset
known_third_party =alembic,apispec,backoff,bleach,cachelib,celery,click,colorama,contextlib2,cron_descriptor,croniter,cryptography,dateutil,deprecation,flask,flask_appbuilder,flask_babel,flask_caching,flask_compress,flask_jwt_extended,flask_login,flask_migrate,flask_sqlalchemy,flask_talisman,flask_testing,flask_wtf,freezegun,geohash,geopy,graphlib,holidays,humanize,isodate,jinja2,jwt,markdown,markupsafe,marshmallow,marshmallow_enum,msgpack,numpy,pandas,parameterized,parsedatetime,pathlib2,pgsanity,pkg_resources,polyline,prison,progress,pyarrow,pyhive,pyparsing,pytest,pytz,redis,requests,retry,selenium,setuptools,simplejson,slack,sqlalchemy,sqlalchemy_utils,sqlparse,typing_extensions,werkzeug,wtforms,wtforms_json,yaml
known_third_party =alembic,apispec,backoff,bleach,cachelib,celery,click,colorama,contextlib2,cron_descriptor,croniter,cryptography,dateutil,deprecation,flask,flask_appbuilder,flask_babel,flask_caching,flask_compress,flask_jwt_extended,flask_login,flask_migrate,flask_sqlalchemy,flask_talisman,flask_testing,flask_wtf,freezegun,geohash,geopy,graphlib,holidays,humanize,isodate,jinja2,jwt,markdown,markupsafe,marshmallow,marshmallow_enum,msgpack,numpy,pandas,parameterized,parsedatetime,pathlib2,pgsanity,pkg_resources,polyline,prison,progress,pyarrow,pyhive,pyparsing,pytest,pytz,redis,requests,selenium,setuptools,simplejson,slack,sqlalchemy,sqlalchemy_utils,sqlparse,typing_extensions,werkzeug,wtforms,wtforms_json,yaml
multi_line_output = 3
order_by_type = false

Expand Down
1 change: 0 additions & 1 deletion setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,6 @@ def get_git_sha():
"pyyaml>=5.4",
"PyJWT>=1.7.1, <2",
"redis",
"retry>=0.9.2",
"selenium>=3.141.0",
"simplejson>=3.15.0",
"slackclient==2.5.0", # PINNED! slack changes file upload api in the future versions
Expand Down
4 changes: 2 additions & 2 deletions superset/reports/notifications/slack.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@
from io import IOBase
from typing import Optional, Union

import backoff
from flask_babel import gettext as __
from retry.api import retry
from slack import WebClient
from slack.errors import SlackApiError, SlackClientError

Expand Down Expand Up @@ -79,7 +79,7 @@ def _get_inline_file(self) -> Optional[Union[str, IOBase, bytes]]:
return self._content.screenshot
return None

@retry(SlackApiError, delay=10, backoff=2, tries=5)
@backoff.on_exception(backoff.expo, SlackApiError, factor=10, base=2, max_tries=5)
def send(self) -> None:
file = self._get_inline_file()
title = self._content.name
Expand Down
13 changes: 8 additions & 5 deletions superset/tasks/schedules.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@
from dateutil.tz import tzlocal
from flask import current_app, render_template, url_for
from flask_babel import gettext as __
from retry.api import retry_call
from selenium.common.exceptions import WebDriverException
from selenium.webdriver import chrome, firefox
from selenium.webdriver.remote.webdriver import WebDriver
Expand All @@ -69,6 +68,7 @@
from superset.tasks.slack_util import deliver_slack_msg
from superset.utils.celery import session_scope
from superset.utils.core import get_email_address_list, send_email_smtp
from superset.utils.retries import retry_call
from superset.utils.screenshots import ChartScreenshot, WebDriverProxy
from superset.utils.urls import get_url_path

Expand Down Expand Up @@ -230,7 +230,7 @@ def destroy_webdriver(
# This is some very flaky code in selenium. Hence the retries
# and catch-all exceptions
try:
retry_call(driver.close, tries=2)
retry_call(driver.close, max_tries=2)
except Exception: # pylint: disable=broad-except
pass
try:
Expand Down Expand Up @@ -271,7 +271,10 @@ def deliver_dashboard( # pylint: disable=too-many-locals
# This is buggy in certain selenium versions with firefox driver
get_element = getattr(driver, "find_element_by_class_name")
element = retry_call(
get_element, fargs=["grid-container"], tries=2, delay=EMAIL_PAGE_RENDER_WAIT
get_element,
fargs=["grid-container"],
max_tries=2,
interval=EMAIL_PAGE_RENDER_WAIT,
)

try:
Expand Down Expand Up @@ -420,8 +423,8 @@ def _get_slice_visualization(
element = retry_call(
driver.find_element_by_class_name,
fargs=["chart-container"],
tries=2,
delay=EMAIL_PAGE_RENDER_WAIT,
max_tries=2,
interval=EMAIL_PAGE_RENDER_WAIT,
)

try:
Expand Down
4 changes: 2 additions & 2 deletions superset/tasks/slack_util.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@
from io import IOBase
from typing import cast, Optional, Union

import backoff
from flask import current_app
from retry.api import retry
from slack import WebClient
from slack.errors import SlackApiError
from slack.web.slack_response import SlackResponse
Expand All @@ -31,7 +31,7 @@
logger = logging.getLogger("tasks.slack_util")


@retry(SlackApiError, delay=10, backoff=2, tries=5)
@backoff.on_exception(backoff.expo, SlackApiError, factor=10, base=2, max_tries=5)
def deliver_slack_msg(
slack_channel: str,
subject: str,
Expand Down
38 changes: 38 additions & 0 deletions superset/utils/retries.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
# Licensed to the Apache Software Foundation (ASF) under one
# or more contributor license agreements. See the NOTICE file
# distributed with this work for additional information
# regarding copyright ownership. The ASF licenses this file
# to you under the Apache License, Version 2.0 (the
# "License"); you may not use this file except in compliance
# with the License. You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing,
# software distributed under the License is distributed on an
# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.

from typing import Any, Callable, Dict, Generator, List, Optional, Type

import backoff


def retry_call(
func: Callable[..., Any],
*args: Any,
strategy: Callable[..., Generator[int, None, None]] = backoff.constant,
exception: Type[Exception] = Exception,
fargs: Optional[List[Any]] = None,
fkwargs: Optional[Dict[str, Any]] = None,
**kwargs: Any
) -> Any:
"""
Retry a given call.
"""
decorated = backoff.on_exception(strategy, exception, *args, **kwargs)(func)
fargs = fargs or []
fkwargs = fkwargs or {}
return decorated(*fargs, **fkwargs)
4 changes: 2 additions & 2 deletions superset/utils/webdriver.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
from typing import Any, Dict, Optional, Tuple, TYPE_CHECKING

from flask import current_app
from retry.api import retry_call
from selenium.common.exceptions import (
StaleElementReferenceException,
TimeoutException,
Expand All @@ -33,6 +32,7 @@
from selenium.webdriver.support.ui import WebDriverWait

from superset.extensions import machine_auth_provider_factory
from superset.utils.retries import retry_call

WindowSize = Tuple[int, int]
logger = logging.getLogger(__name__)
Expand Down Expand Up @@ -85,7 +85,7 @@ def destroy(driver: WebDriver, tries: int = 2) -> None:
# This is some very flaky code in selenium. Hence the retries
# and catch-all exceptions
try:
retry_call(driver.close, tries=tries)
retry_call(driver.close, max_tries=tries)
except Exception: # pylint: disable=broad-except
pass
try:
Expand Down

0 comments on commit c9dad05

Please sign in to comment.