Factorize getting the request parameters length and offset (#1934)

* Extract function get_request_parameter_offset * Align offset error messages * Move get_request_parameter_offset to libapi * Extract function get_request_parameter_length * Align length error messages * Add too large length error to openapi.json * Move get_request_parameter_length to libapi
huggingface · Oct 9, 2023 · 487b578 · 487b578
1 parent 54ebe02
commit 487b578
Show file tree

Hide file tree

Showing 5 changed files with 51 additions and 38 deletions.
diff --git a/docs/source/openapi.json b/docs/source/openapi.json
@@ -2868,13 +2868,19 @@
                   "negative-offset": {
                     "summary": "The offset must be positive.",
                     "value": {
-                      "error": "Offset must be positive"
+                      "error": "Parameter 'offset' must be positive"
                     }
                   },
                   "negative-length": {
                     "summary": "The length must be positive.",
                     "value": {
-                      "error": "Length must be positive"
+                      "error": "Parameter 'length' must be positive"
+                    }
+                  },
+                  "too-large-length": {
+                    "summary": "The length must not be too large.",
+                    "value": {
+                      "error": "Parameter 'length' must not be greater than 100"
                     }
                   }
                 }
@@ -3227,13 +3233,19 @@
                   "negative-offset": {
                     "summary": "The offset must be positive.",
                     "value": {
-                      "error": "Offset must be positive"
+                      "error": "Parameter 'offset' must be positive"
                     }
                   },
                   "negative-length": {
                     "summary": "The length must be positive.",
                     "value": {
-                      "error": "Length must be positive"
+                      "error": "Parameter 'length' must be positive"
+                    }
+                  },
+                  "too-large-length": {
+                    "summary": "The length must not be too large.",
+                    "value": {
+                      "error": "Parameter 'length' must not be greater than 100"
                     }
                   }
                 }
@@ -3839,6 +3851,12 @@
                     "value": {
                       "error": "Parameter 'length' must be positive"
                     }
+                  },
+                  "too-large-length": {
+                    "summary": "The length must not be too large.",
+                    "value": {
+                      "error": "Parameter 'length' must not be greater than 100"
+                    }
                   }
                 }
               }

diff --git a/libs/libapi/src/libapi/request.py b/libs/libapi/src/libapi/request.py
@@ -0,0 +1,20 @@
+from libcommon.utils import MAX_NUM_ROWS_PER_PAGE
+from starlette.requests import Request
+
+from libapi.exceptions import InvalidParameterError
+
+
+def get_request_parameter_length(request: Request) -> int:
+    length = int(request.query_params.get("length", MAX_NUM_ROWS_PER_PAGE))
+    if length < 0:
+        raise InvalidParameterError("Parameter 'length' must be positive")
+    elif length > MAX_NUM_ROWS_PER_PAGE:
+        raise InvalidParameterError(f"Parameter 'length' must not be greater than {MAX_NUM_ROWS_PER_PAGE}")
+    return length
+
+
+def get_request_parameter_offset(request: Request) -> int:
+    offset = int(request.query_params.get("offset", 0))
+    if offset < 0:
+        raise InvalidParameterError(message="Parameter 'offset' must be positive")
+    return offset
diff --git a/services/rows/src/rows/routes/rows.py b/services/rows/src/rows/routes/rows.py
@@ -9,10 +9,10 @@
 from libapi.authentication import auth_check
 from libapi.exceptions import (
     ApiError,
-    InvalidParameterError,
     MissingRequiredParameterError,
     UnexpectedApiError,
 )
+from libapi.request import get_request_parameter_length, get_request_parameter_offset
 from libapi.response import create_response
 from libapi.utils import (
     Endpoint,
@@ -29,7 +29,6 @@
 from libcommon.s3_client import S3Client
 from libcommon.simple_cache import CachedArtifactError, CachedArtifactNotFoundError
 from libcommon.storage import StrPath
-from libcommon.utils import MAX_NUM_ROWS_PER_PAGE
 from libcommon.viewer_utils.asset import update_last_modified_date_of_rows_in_assets_dir
 from libcommon.viewer_utils.features import UNSUPPORTED_FEATURES
 from starlette.requests import Request
@@ -85,16 +84,8 @@ async def rows_endpoint(request: Request) -> Response:
                     split = request.query_params.get("split")
                     if not dataset or not config or not split or not are_valid_parameters([dataset, config, split]):
                         raise MissingRequiredParameterError("Parameter 'dataset', 'config' and 'split' are required")
-                    offset = int(request.query_params.get("offset", 0))
-                    if offset < 0:
-                        raise InvalidParameterError(message="Offset must be positive")
-                    length = int(request.query_params.get("length", MAX_NUM_ROWS_PER_PAGE))
-                    if length < 0:
-                        raise InvalidParameterError("Length must be positive")
-                    if length > MAX_NUM_ROWS_PER_PAGE:
-                        raise InvalidParameterError(
-                            f"Parameter 'length' must not be bigger than {MAX_NUM_ROWS_PER_PAGE}"
-                        )
+                    offset = get_request_parameter_offset(request)
+                    length = get_request_parameter_length(request)
                     logging.info(
                         f"/rows, dataset={dataset}, config={config}, split={split}, offset={offset}, length={length}"
                     )

diff --git a/services/search/src/search/routes/filter.py b/services/search/src/search/routes/filter.py
@@ -21,6 +21,7 @@
     MissingRequiredParameterError,
     UnexpectedApiError,
 )
+from libapi.request import get_request_parameter_length, get_request_parameter_offset
 from libapi.response import ROW_IDX_COLUMN, create_response
 from libapi.utils import (
     Endpoint,
@@ -34,7 +35,6 @@
 from libcommon.prometheus import StepProfiler
 from libcommon.s3_client import S3Client
 from libcommon.storage import StrPath
-from libcommon.utils import MAX_NUM_ROWS_PER_PAGE
 from libcommon.viewer_utils.features import get_supported_unsupported_columns
 from starlette.requests import Request
 from starlette.responses import Response
@@ -100,16 +100,8 @@ async def filter_endpoint(request: Request) -> Response:
                             "Parameters 'dataset', 'config', 'split' and 'where' are required"
                         )
                     validate_where_parameter(where)
-                    offset = int(request.query_params.get("offset", 0))
-                    if offset < 0:
-                        raise InvalidParameterError(message="Parameter 'offset' must be positive")
-                    length = int(request.query_params.get("length", MAX_NUM_ROWS_PER_PAGE))
-                    if length < 0:
-                        raise InvalidParameterError("Parameter 'length' must be positive")
-                    elif length > MAX_NUM_ROWS_PER_PAGE:
-                        raise InvalidParameterError(
-                            f"Parameter 'length' must not be bigger than {MAX_NUM_ROWS_PER_PAGE}"
-                        )
+                    offset = get_request_parameter_offset(request)
+                    length = get_request_parameter_length(request)
                     logger.info(
                         f'/filter, dataset={dataset}, config={config}, split={split}, where="{where}",'
                         f" offset={offset}, length={length}"

diff --git a/services/search/src/search/routes/search.py b/services/search/src/search/routes/search.py
@@ -16,11 +16,11 @@
 )
 from libapi.exceptions import (
     ApiError,
-    InvalidParameterError,
     MissingRequiredParameterError,
     SearchFeatureNotAvailableError,
     UnexpectedApiError,
 )
+from libapi.request import get_request_parameter_length, get_request_parameter_offset
 from libapi.response import ROW_IDX_COLUMN
 from libapi.utils import (
     Endpoint,
@@ -162,17 +162,9 @@ async def search_endpoint(request: Request) -> Response:
                             "Parameter 'dataset', 'config', 'split' and 'query' are required"
                         )
 
-                    offset = int(request.query_params.get("offset", 0))
-                    if offset < 0:
-                        raise InvalidParameterError(message="Offset must be positive")
+                    offset = get_request_parameter_offset(request)
 
-                    length = int(request.query_params.get("length", MAX_NUM_ROWS_PER_PAGE))
-                    if length < 0:
-                        raise InvalidParameterError("Length must be positive")
-                    if length > MAX_NUM_ROWS_PER_PAGE:
-                        raise InvalidParameterError(
-                            f"Parameter 'length' must not be bigger than {MAX_NUM_ROWS_PER_PAGE}"
-                        )
+                    length = get_request_parameter_length(request)
 
                 with StepProfiler(method="search_endpoint", step="check authentication"):
                     # if auth_check fails, it will raise an exception that will be caught below