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.

Sign up for GitHub

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

Jump to bottom

feat: Added function to generate error dict #51

Merged
merged 2 commits into from
Sep 23, 2024
Merged

feat: Added function to generate error dict #51

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6798ea8
feat: Added function to generate error dict
Seluj78 Sep 23, 2024
d0a9653
doc: Fixed small error in documentation for register_error_handlers
Seluj78 Sep 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion docs/source/api.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ Private API
.. autofunction:: flask_utils.decorators._is_allow_empty
.. autofunction:: flask_utils.decorators._check_type

.. autofunction:: flask_utils.errors._error_template._generate_error_json
.. autofunction:: flask_utils.errors._error_template._generate_error_dict
.. autofunction:: flask_utils.errors._error_template._generate_error_response
Comment on lines +59 to +60
Copy link
Contributor

Choose a reason for hiding this comment

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

Tip

Codebase Verification

Incomplete renaming of _generate_error_json detected.

  • Remaining instances of _generate_error_json:
    • flask_utils/errors/_error_template.py:
      • Imports and function calls to _generate_error_json.
    • tests/test_generate_error_dict.py:
      • References to _generate_error_json.
    • docs/source/api.rst:
      • Documentation references to _generate_error_json.

Please ensure that all instances of _generate_error_json are renamed to _generate_error_dict to maintain consistency and prevent potential issues.

Analysis chain

LGTM! Consider adding brief descriptions.

The addition of _generate_error_dict and _generate_error_response functions to the Private API documentation aligns with the PR objectives. Good job on maintaining the documentation structure and following the naming convention for private functions.

Consider adding a brief description for each function to improve the documentation. For example:

.. autofunction:: flask_utils.errors._error_template._generate_error_dict
   :description: Generates a dictionary representation of an error.

.. autofunction:: flask_utils.errors._error_template._generate_error_response
   :description: Generates an error response object.

The AI summary mentioned that _generate_error_json was renamed to _generate_error_dict. Let's verify this change:

This will help ensure that the renaming was done consistently across the codebase.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the renaming of _generate_error_json to _generate_error_dict

# Test 1: Check for any remaining instances of _generate_error_json
echo "Checking for remaining instances of _generate_error_json:"
rg "_generate_error_json" --type rst --type python

# Test 2: Verify the presence of _generate_error_dict
echo "Verifying the presence of _generate_error_dict:"
rg "_generate_error_dict" --type rst --type python

Length of output: 393

Script:

#!/bin/bash
# Description: Verify the renaming of _generate_error_json to _generate_error_dict by searching relevant file extensions

# Test 1: Check for any remaining instances of _generate_error_json in .rst and .py files
echo "Checking for remaining instances of _generate_error_json:"
rg "_generate_error_json" --type rst --type py

# Test 2: Verify the presence of _generate_error_dict in .rst and .py files
echo "Verifying the presence of _generate_error_dict:"
rg "_generate_error_dict" --type rst --type py

Length of output: 1405


.. autofunction:: flask_utils.errors._register_error_handlers
2 changes: 1 addition & 1 deletion flask_utils/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# Increment versions here according to SemVer
__version__ = "0.7.2"
__version__ = "0.8.0"

from flask_utils.utils import is_it_true
from flask_utils.errors import GoneError
Expand Down
26 changes: 13 additions & 13 deletions flask_utils/errors/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
from flask_utils.errors.forbidden import ForbiddenError
from flask_utils.errors.badrequest import BadRequestError
from flask_utils.errors.unauthorized import UnauthorizedError
from flask_utils.errors._error_template import _generate_error_json
from flask_utils.errors._error_template import _generate_error_response
from flask_utils.errors.failed_dependency import FailedDependencyError
from flask_utils.errors.web_server_is_down import WebServerIsDownError
from flask_utils.errors.service_unavailable import ServiceUnavailableError
Expand Down Expand Up @@ -58,7 +58,7 @@ def generate_badrequest(error: BadRequestError) -> Response:
:return: Returns the response formatted
:rtype: flask.Response
"""
return _generate_error_json(error, 400)
return _generate_error_response(error)

@application.errorhandler(ConflictError)
def generate_conflict(error: ConflictError) -> Response:
Expand All @@ -73,7 +73,7 @@ def generate_conflict(error: ConflictError) -> Response:
:rtype: flask.Response
"""

return _generate_error_json(error, 409)
return _generate_error_response(error)

@application.errorhandler(ForbiddenError)
def generate_forbidden(error: ForbiddenError) -> Response:
Expand All @@ -88,7 +88,7 @@ def generate_forbidden(error: ForbiddenError) -> Response:
:rtype: flask.Response
"""

return _generate_error_json(error, 403)
return _generate_error_response(error)

@application.errorhandler(NotFoundError)
def generate_notfound(error: NotFoundError) -> Response:
Expand All @@ -103,7 +103,7 @@ def generate_notfound(error: NotFoundError) -> Response:
:rtype: flask.Response
"""

return _generate_error_json(error, 404)
return _generate_error_response(error)

@application.errorhandler(UnauthorizedError)
def generate_unauthorized(error: UnauthorizedError) -> Response:
Expand All @@ -118,7 +118,7 @@ def generate_unauthorized(error: UnauthorizedError) -> Response:
:rtype: flask.Response
"""

return _generate_error_json(error, 401)
return _generate_error_response(error)

@application.errorhandler(OriginIsUnreachableError)
def generate_origin_is_unreachable(error: OriginIsUnreachableError) -> Response:
Expand All @@ -133,7 +133,7 @@ def generate_origin_is_unreachable(error: OriginIsUnreachableError) -> Response:
:rtype: flask.Response
"""

return _generate_error_json(error, 523)
return _generate_error_response(error)

@application.errorhandler(WebServerIsDownError)
def generate_web_server_is_down(error: WebServerIsDownError) -> Response:
Expand All @@ -148,7 +148,7 @@ def generate_web_server_is_down(error: WebServerIsDownError) -> Response:
:rtype: flask.Response
"""

return _generate_error_json(error, 521)
return _generate_error_response(error)

@application.errorhandler(FailedDependencyError)
def generate_failed_dependency(error: FailedDependencyError) -> Response:
Expand All @@ -163,7 +163,7 @@ def generate_failed_dependency(error: FailedDependencyError) -> Response:
:rtype: flask.Response
"""

return _generate_error_json(error, 424)
return _generate_error_response(error)

@application.errorhandler(GoneError)
def generate_gone(error: GoneError) -> Response:
Expand All @@ -178,7 +178,7 @@ def generate_gone(error: GoneError) -> Response:
:rtype: flask.Response
"""

return _generate_error_json(error, 410)
return _generate_error_response(error)

@application.errorhandler(UnprocessableEntityError)
def generate_unprocessable_entity(error: UnprocessableEntityError) -> Response:
Expand All @@ -193,7 +193,7 @@ def generate_unprocessable_entity(error: UnprocessableEntityError) -> Response:
:rtype: flask.Response
"""

return _generate_error_json(error, 422)
return _generate_error_response(error)

@application.errorhandler(ServiceUnavailableError)
def generate_service_unavailable(error: ServiceUnavailableError) -> Response:
Expand All @@ -208,7 +208,7 @@ def generate_service_unavailable(error: ServiceUnavailableError) -> Response:
:rtype: flask.Response
"""

return _generate_error_json(error, 503)
return _generate_error_response(error)


__all__ = [
Expand All @@ -217,7 +217,7 @@ def generate_service_unavailable(error: ServiceUnavailableError) -> Response:
"ForbiddenError",
"NotFoundError",
"UnauthorizedError",
"_generate_error_json",
"_generate_error_response",
"FailedDependencyError",
"WebServerIsDownError",
"OriginIsUnreachableError",
Expand Down
72 changes: 56 additions & 16 deletions flask_utils/errors/_error_template.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,18 +1,63 @@
from typing import Any
from typing import Dict

from flask import Response
from flask import jsonify

from flask_utils.errors.base_class import _BaseFlaskException


def _generate_error_json(error: _BaseFlaskException, status_code: int) -> Response:
def _generate_error_dict(error: _BaseFlaskException) -> Dict[str, Any]:
"""
This function is used to generate a json of the error passed
This function is used to generate a dict of the error passed

:param error: The error containing the message and solution
:type error: _BaseFlaskException

:param status_code: The status code of the error.
:type status_code: int
:return: Returns a dict containing a json representation of the error
:rtype: dict
Copy link
Contributor

Choose a reason for hiding this comment

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

Align :rtype: in docstring with return type annotation

The :rtype: in the docstring specifies dict, but the function's return type annotation is Dict[str, Any]. For consistency and clarity, consider updating the :rtype: to Dict[str, Any].

Apply this diff to adjust the docstring:

-    :rtype: dict
+    :rtype: Dict[str, Any]
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
:rtype: dict
:rtype: Dict[str, Any]


:Example:

.. code-block:: python

from flask_utils import ConflictError # or any other error
from flask_utils.errors._error_template import _generate_error_json

json = _generate_error_json(error)
Copy link
Contributor

Choose a reason for hiding this comment

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

Update function references in _generate_error_dict docstring example

The example code in the docstring still uses the old function name _generate_error_json instead of the updated _generate_error_dict. This could cause confusion for users referring to the documentation.

Apply this diff to correct the function name in the example and ensure consistency:

         from flask_utils import ConflictError  # or any other error
-        from flask_utils.errors._error_template import _generate_error_json
+        from flask_utils.errors._error_template import _generate_error_dict

-        json = _generate_error_json(error)
+        json = _generate_error_dict(error)

Also applies to: 43-52

# Sample output:
# {
# "success": False,
# "error": {
# "type": "ConflictError",
# "name": "ConflictError",
# "message": "This is the message",
# "solution": "This is the solution"
# },
# "code": 409
# }

.. versionadded:: 0.8.0
"""

return {
"success": False,
"error": {
"type": error.__class__.__name__,
"name": error.name,
"message": error.msg,
"solution": error.solution,
},
"code": error.status_code,
}


def _generate_error_response(error: _BaseFlaskException) -> Response:
"""
This function is used to generate a json of the error passed

:param error: The error containing the message and solution
:type error: _BaseFlaskException

:return: Returns a json containing all the info
:rtype: flask.Response
Expand All @@ -34,19 +79,14 @@ class MyError(_BaseFlaskException):

json = _generate_error_json(error, 666)

.. versionchanged:: 0.8.0
This function was renamed from ``generate_error_json`` to ``_generate_error_response``.
It now returns a ``flask.Response`` object, calling
:func:`~flask_utils.errors._error_template._generate_error_json` to generate the json.
Copy link
Contributor

Choose a reason for hiding this comment

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

Correct function reference in versionchanged section

In the .. versionchanged:: 0.8.0 section, the function _generate_error_json is still referenced. This should be updated to _generate_error_dict to accurately reflect the changes.

Apply this diff to fix the function reference:

         It now returns a ``flask.Response`` object, calling
-        :func:`~flask_utils.errors._error_template._generate_error_json` to generate the json.
+        :func:`~flask_utils.errors._error_template._generate_error_dict` to generate the error dictionary.
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
This function was renamed from ``generate_error_json`` to ``_generate_error_response``.
It now returns a ``flask.Response`` object, calling
:func:`~flask_utils.errors._error_template._generate_error_json` to generate the json.
This function was renamed from ``generate_error_json`` to ``_generate_error_response``.
It now returns a ``flask.Response`` object, calling
:func:`~flask_utils.errors._error_template._generate_error_dict` to generate the error dictionary.


.. versionadded:: 0.1.0
"""
success = False
json = {
"success": success,
"error": {
"type": error.__class__.__name__,
"name": error.name,
"message": error.msg,
"solution": error.solution,
},
"code": status_code,
}
json = _generate_error_dict(error)
resp: Response = jsonify(json)
resp.status_code = status_code
resp.status_code = error.status_code
return resp
23 changes: 13 additions & 10 deletions flask_utils/errors/base_class.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,17 @@ class _BaseFlaskException(Exception):
"""
This is the base class for all the exceptions in this package.

:param msg: The message to be displayed in the error.
:param name: The name of the error
:type name: str

:param msg: The message to be displayed
:type msg: str
:param solution: The solution to the error.
:type solution: Optional[str]
:param status_code: The status code of the error.
:type status_code: Optional[int]
:param name: The name of the error.
:type name: Optional[str]

:param solution: The solution to the problem
:type solution: str

:param status_code: The status code to be returned
:type status_code: int

:Example:

Expand All @@ -29,7 +32,7 @@ class MyError(_BaseFlaskException):
.. versionadded:: 0.1.0
"""

name: Optional[str] = None
msg: Optional[str] = None
name: str = "BaseError"
msg: str = "An error occurred"
solution: Optional[str] = "Try again."
status_code: Optional[int] = 400
status_code: int = 400
Comment on lines +35 to +38
Copy link
Contributor

Choose a reason for hiding this comment

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

Inconsistency between attributes and documentation

There's an inconsistency between the attribute definitions and the documentation:

  1. The solution attribute is still defined as Optional[str], but the documentation now specifies it as str.
  2. The status_code attribute is correctly updated to int, aligning with the documentation.

To resolve this inconsistency, consider updating the solution attribute as follows:

-    solution: Optional[str] = "Try again."
+    solution: str = "Try again."

This change will make the attribute definition consistent with the updated documentation.

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
name: str = "BaseError"
msg: str = "An error occurred"
solution: Optional[str] = "Try again."
status_code: Optional[int] = 400
status_code: int = 400
name: str = "BaseError"
msg: str = "An error occurred"
solution: str = "Try again."
status_code: int = 400

45 changes: 45 additions & 0 deletions tests/test_generate_error_dict.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
import pytest

from flask_utils.errors import GoneError
from flask_utils.errors import ConflictError
from flask_utils.errors import NotFoundError
from flask_utils.errors import ForbiddenError
from flask_utils.errors import BadRequestError
from flask_utils.errors import UnauthorizedError
from flask_utils.errors import WebServerIsDownError
from flask_utils.errors import FailedDependencyError
from flask_utils.errors import ServiceUnavailableError
from flask_utils.errors import OriginIsUnreachableError
from flask_utils.errors import UnprocessableEntityError


class TestGenerateErrorDict:
@pytest.mark.parametrize(
"error",
[
ConflictError("This is the message", "This is the solution"),
GoneError("This is the message", "This is the solution"),
ForbiddenError("This is the message", "This is the solution"),
UnauthorizedError("This is the message", "This is the solution"),
NotFoundError("This is the message", "This is the solution"),
BadRequestError("This is the message", "This is the solution"),
UnprocessableEntityError("This is the message", "This is the solution"),
FailedDependencyError("This is the message", "This is the solution"),
ServiceUnavailableError("This is the message", "This is the solution"),
OriginIsUnreachableError("This is the message", "This is the solution"),
WebServerIsDownError("This is the message", "This is the solution"),
],
)
def test_generate_error_dict(self, error):
from flask_utils.errors._error_template import _generate_error_dict

assert _generate_error_dict(error) == {
"success": False,
"error": {
"type": error.__class__.__name__,
"name": error.name,
"message": error.msg,
"solution": error.solution,
},
"code": error.status_code,
}
Comment on lines +33 to +45
Copy link
Contributor

Choose a reason for hiding this comment

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

Effective test implementation with a minor optimization opportunity.

The test method effectively validates the _generate_error_dict function's output against the expected error dictionary structure. All relevant fields are checked, ensuring comprehensive validation.

Consider moving the import statement outside the test method to improve performance, especially when running multiple tests:

from flask_utils.errors._error_template import _generate_error_dict

class TestGenerateErrorDict:
    # ... (existing code) ...

    def test_generate_error_dict(self, error):
        assert _generate_error_dict(error) == {
            # ... (existing assertion) ...
        }

This change would prevent repeated imports across multiple test runs, potentially improving test execution time.

Loading