Skip to content

feat(integrations): added django integration #10

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ghettoDdOS
Copy link

Pull Request

#9 Closes

What’s Included?

  • New feature
  • Bug fix
  • Documentation improvement
  • Refactor / internal change

Copy link
Owner

@mr-fatalyst mr-fatalyst left a comment

Choose a reason for hiding this comment

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

Thank you very much for the great work!
Unfortunately, I noticed your PR a bit too late and had already committed some changes.
I hope there are no serious conflicts.
I found a few minor issues. If you have time, could you please address them.
Thanks again.

@@ -0,0 +1,117 @@
import json
Copy link
Owner

Choose a reason for hiding this comment

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

I removed the use of this library. Please use pydantic_core instead.


outer = self

@csrf_exempt
Copy link
Owner

Choose a reason for hiding this comment

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

I think it will block access from Swagger. It should be something like this:

    def _create_or_update_view(self, path: str, method: str, endpoint: Callable):
        view = self._views.get(path)
        if not view:
            view = type("DynamicView", (View,), {
                "dispatch": csrf_exempt(View.dispatch)
            })
            self._views[path] = view
        method_name = method.lower()

        outer = self

        def handle(self, req, **path_params):
            return outer._handle_request(endpoint, req, **path_params)

        setattr(view, method_name, handle)
        return view


assert response.status_code == 200
assert "text/html" in response.headers["content-type"]
assert "redoc" in response.text
Copy link
Owner

@mr-fatalyst mr-fatalyst Apr 25, 2025

Choose a reason for hiding this comment

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

You used HttpResponse in _handle_request. This class does not have text. It has to be content with .decode().

except Exception as e:
error_response = self.handle_exception(e)
return JsonResponse(error_response, status=getattr(e, "status_code", 500))
result = self._serialize_response(result)
Copy link
Owner

Choose a reason for hiding this comment

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

I think it would be better to use JsonResponse instead.

result = self._serialize_response(result)
response = HttpResponse(status=status_code)
if result:
    json.dump(result, response)
    response.headers["Content-Type"] = "application/json"
return response

should be changed to something like this:

result = self._serialize_response(result)
return JsonResponse(result if result is not None else {}, status=status_code)

What do you think?

@router.get(
"/posts/", tags=["Posts"], response_errors=[500], response_model=list[PostSchema]
)
async def get_posts(body: FilterPostSchema) -> list[PostSchema]:
Copy link
Owner

Choose a reason for hiding this comment

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

DjangoRouter can't work with async.

@router.delete(
"/posts/{post_id}", tags=["Posts"], status_code=204, response_errors=[400, 404, 500]
)
async def delete_post(post_id: int) -> None:
Copy link
Owner

Choose a reason for hiding this comment

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

the same

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants