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

♻️ improve error handling of pricing plans in webserver #4980

2 changes: 1 addition & 1 deletion .env-devel
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ TRAEFIK_SIMCORE_ZONE=internal_simcore_stack

# NOTE: WEBSERVER_SESSION_SECRET_KEY = $(python3 -c "from cryptography.fernet import Fernet; print(Fernet.generate_key())")

WEBSERVER_CREDIT_COMPUTATION_ENABLED=0
WEBSERVER_CREDIT_COMPUTATION_ENABLED=1
matusdrobuliak66 marked this conversation as resolved.
Show resolved Hide resolved

WEBSERVER_LOGLEVEL=INFO

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ async def on_payment_completed(
credit_transaction_id = await rut_api.create_credit_transaction(
product_name=transaction.product_name,
wallet_id=transaction.wallet_id,
wallet_name="id={transaction.wallet_id}",
wallet_name=f"id={transaction.wallet_id}",
matusdrobuliak66 marked this conversation as resolved.
Show resolved Hide resolved
user_id=transaction.user_id,
user_email=transaction.user_email,
osparc_credits=transaction.osparc_credits,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,4 +19,4 @@ def http404_error_handler(
request: Request, # pylint: disable=unused-argument
error: CustomResourceUsageTrackerError,
) -> JSONResponse:
return JSONResponse(status_code=404, content={"message": error.msg_template})
return JSONResponse(status_code=404, content={"message": f"{error.msg_template}"})
Original file line number Diff line number Diff line change
Expand Up @@ -690,7 +690,7 @@ async def get_pricing_unit(
row = result.first()
if row is None:
raise CustomResourceUsageTrackerError(
msg=f"Pricing unit id {pricing_unit_id} not found"
msg=f"Pricing plan {pricing_plan_id} and pricing unit {pricing_unit_id} for product {product_name} not found"
)
return PricingUnitsDB.from_orm(row)

Expand Down Expand Up @@ -723,6 +723,6 @@ async def get_pricing_unit_cost_by_id(
row = result.first()
if row is None:
raise CustomResourceUsageTrackerError(
msg=f"Pricing unit cosd id {pricing_unit_cost_id} not found in the resource_tracker_pricing_unit_costs table",
msg=f"Pricing unit cost id {pricing_unit_cost_id} not found in the resource_tracker_pricing_unit_costs table",
)
return PricingUnitCostsDB.from_orm(row)
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,5 @@
MSG_CATALOG_SERVICE_UNAVAILABLE: Final[
str
] = "Currently catalog service is unavailable, please try again later"

MSG_CATALOG_SERVICE_NOT_FOUND: Final[str] = "Not Found"
matusdrobuliak66 marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
from yarl import URL

from .._meta import api_version_prefix
from ._constants import MSG_CATALOG_SERVICE_UNAVAILABLE
from ._constants import MSG_CATALOG_SERVICE_NOT_FOUND, MSG_CATALOG_SERVICE_UNAVAILABLE
from .settings import get_plugin_settings

_logger = logging.getLogger(__name__)
Expand All @@ -39,7 +39,14 @@ def _handle_client_exceptions(app: web.Application) -> Iterator[ClientSession]:

yield session

except (asyncio.TimeoutError, ClientConnectionError, ClientResponseError) as err:
except ClientResponseError as err:
if err.status == 404:
matusdrobuliak66 marked this conversation as resolved.
Show resolved Hide resolved
raise web.HTTPNotFound(reason=MSG_CATALOG_SERVICE_NOT_FOUND)
raise web.HTTPServiceUnavailable(
reason=MSG_CATALOG_SERVICE_UNAVAILABLE
) from err

except (asyncio.TimeoutError, ClientConnectionError) as err:
_logger.debug("Request to catalog service failed: %s", err)
raise web.HTTPServiceUnavailable(
reason=MSG_CATALOG_SERVICE_UNAVAILABLE
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ async def get_default_service_pricing_plan(
)
with handle_client_exceptions(app) as session:
async with session.get(url) as response:
response.raise_for_status()
body: dict = await response.json()
return parse_obj_as(ServicePricingPlanGet, body)

Expand All @@ -119,6 +120,7 @@ async def get_pricing_plan_unit(
)
with handle_client_exceptions(app) as session:
async with session.get(url) as response:
response.raise_for_status()
body: dict = await response.json()
return parse_obj_as(PricingUnitGet, body)

Expand All @@ -139,6 +141,7 @@ async def sum_total_available_credits_in_the_wallet(
)
with handle_client_exceptions(app) as session:
async with session.post(url) as response:
response.raise_for_status()
matusdrobuliak66 marked this conversation as resolved.
Show resolved Hide resolved
body: dict = await response.json()
return WalletTotalCredits.construct(**body)

Expand Down Expand Up @@ -168,6 +171,7 @@ async def add_credits_to_wallet(
}
with handle_client_exceptions(app) as session:
async with session.post(url, json=body) as response:
response.raise_for_status()
output: dict = await response.json()
return output

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,5 @@
MSG_RESOURCE_USAGE_TRACKER_SERVICE_UNAVAILABLE: Final[
str
] = "Currently resource usage tracker service is unavailable, please try again later"

MSG_RESOURCE_USAGE_TRACKER_NOT_FOUND: Final[str] = "Not Found"
matusdrobuliak66 marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,10 @@
from aiohttp.client_exceptions import ClientConnectionError, ClientResponseError
from servicelib.aiohttp.client_session import get_client_session

from ._constants import MSG_RESOURCE_USAGE_TRACKER_SERVICE_UNAVAILABLE
from ._constants import (
MSG_RESOURCE_USAGE_TRACKER_NOT_FOUND,
MSG_RESOURCE_USAGE_TRACKER_SERVICE_UNAVAILABLE,
)

_logger = logging.getLogger(__name__)

Expand All @@ -18,8 +21,14 @@ def handle_client_exceptions(app: web.Application) -> Iterator[ClientSession]:
session: ClientSession = get_client_session(app)

yield session
except (ClientResponseError) as err:
if err.status == 404:
raise web.HTTPNotFound(reason=MSG_RESOURCE_USAGE_TRACKER_NOT_FOUND)
raise web.HTTPServiceUnavailable(
reason=MSG_RESOURCE_USAGE_TRACKER_SERVICE_UNAVAILABLE
) from err

except (asyncio.TimeoutError, ClientConnectionError, ClientResponseError) as err:
except (asyncio.TimeoutError, ClientConnectionError) as err:
_logger.debug("Request to resource usage tracker service failed: %s", err)
raise web.HTTPServiceUnavailable(
reason=MSG_RESOURCE_USAGE_TRACKER_SERVICE_UNAVAILABLE
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
# pylint:disable=unused-variable
# pylint:disable=unused-argument
# pylint:disable=redefined-outer-name

import re
import urllib.parse

import pytest
from aiohttp import web
from aiohttp.test_utils import TestClient
from models_library.api_schemas_resource_usage_tracker.pricing_plans import (
ServicePricingPlanGet,
)
from models_library.utils.fastapi_encoders import jsonable_encoder
from pydantic import parse_obj_as
from pytest_simcore.aioresponses_mocker import AioResponsesMock
from pytest_simcore.helpers.utils_assert import assert_status
from pytest_simcore.helpers.utils_login import UserInfoDict
from settings_library.resource_usage_tracker import ResourceUsageTrackerSettings
from simcore_service_webserver.db.models import UserRole
from simcore_service_webserver.resource_usage.settings import get_plugin_settings


@pytest.fixture
def mock_rut_api_responses(
client: TestClient, aioresponses_mocker: AioResponsesMock
) -> AioResponsesMock:
assert client.app
settings: ResourceUsageTrackerSettings = get_plugin_settings(client.app)

service_pricing_plan_get = parse_obj_as(
ServicePricingPlanGet,
ServicePricingPlanGet.Config.schema_extra["examples"][0],
)
aioresponses_mocker.get(
re.compile(f"^{settings.api_base_url}/services/+.+$"),
payload=jsonable_encoder(service_pricing_plan_get),
)

return aioresponses_mocker


@pytest.mark.parametrize(
"user_role,expected",
[
(UserRole.ANONYMOUS, web.HTTPUnauthorized),
(UserRole.GUEST, web.HTTPOk),
(UserRole.USER, web.HTTPOk),
(UserRole.TESTER, web.HTTPOk),
],
)
async def test_get_service_pricing_plan(
matusdrobuliak66 marked this conversation as resolved.
Show resolved Hide resolved
client: TestClient,
logged_user: UserInfoDict,
mock_rut_api_responses: AioResponsesMock,
expected: type[web.HTTPException],
):
assert client.app
assert client.app.router
url = client.app.router["get_service_pricing_plan"].url_for(
service_key=urllib.parse.quote("simcore/services/dynamic/someservice", safe=""),
service_version="3.4.5",
)
response = await client.get(f"{url}")
matusdrobuliak66 marked this conversation as resolved.
Show resolved Hide resolved
await assert_status(response, expected)


@pytest.fixture
def mock_catalog_get_service_pricing_plan_not_found(
client: TestClient, aioresponses_mocker: AioResponsesMock
) -> AioResponsesMock:
assert client.app
settings: ResourceUsageTrackerSettings = get_plugin_settings(client.app)
url_pattern = re.compile(f"^{settings.base_url}+/.*$")

aioresponses_mocker.get(url_pattern, exception=web.HTTPNotFound)
matusdrobuliak66 marked this conversation as resolved.
Show resolved Hide resolved
return aioresponses_mocker


@pytest.mark.parametrize(
"user_role,expected",
[
(UserRole.TESTER, web.HTTPNotFound),
],
)
async def test_get_service_pricing_plan_raises_not_found_error(
client: TestClient,
logged_user: UserInfoDict,
mock_catalog_get_service_pricing_plan_not_found: AioResponsesMock,
expected: type[web.HTTPException],
):
assert client.app
assert client.app.router
url = client.app.router["get_service_pricing_plan"].url_for(
service_key="simcore%2Fservices%2Fdynamic%2Fsomeservice",
service_version="3.4.5",
)
response = await client.get(f"{url}")
await assert_status(response, expected)
Loading