Skip to content

Conversation

@gi0baro
Copy link
Contributor

@gi0baro gi0baro commented May 30, 2025

Close #569

@gi0baro
Copy link
Contributor Author

gi0baro commented May 30, 2025

@RobertoPrevato regarding tests, do you have any advice? I don't see any specific tests for the scribe module. Should I just add a test in test_responses for this?

@RobertoPrevato
Copy link
Member

Thanks @gi0baro for contributing! Regarding tests, I didn't include tests specific to the scribe namespace because most functions of the Cython code are defined as cdef for best performance, and cannot be imported from Python code. Yes, I would probably add more tests to test_responses, or add a dedicated module for this feature if that makes sense.

@gi0baro gi0baro force-pushed the 569-asgi-pathsend branch 2 times, most recently from d9c6fab to f781b9d Compare June 27, 2025 12:37
@RobertoPrevato RobertoPrevato marked this pull request as ready for review September 27, 2025 11:57
@RobertoPrevato
Copy link
Member

RobertoPrevato commented Sep 27, 2025

Thanks @gi0baro,
Sorry for taking such a long time to come back to this. I understand your PR is complete and can be merged, correct? I plan to release a new version to PyPi this weekend.

I just tested with Granian and it seems to fail serving files (unless I am doing something wrong). Please let me know if I can help with anything for this PR.

@RobertoPrevato RobertoPrevato marked this pull request as draft September 27, 2025 19:12
@gi0baro
Copy link
Contributor Author

gi0baro commented Sep 29, 2025

Hey @RobertoPrevato

I'm the one who actually feels sorry for this; I hadn't chance to continue working on this since my last comment.
The current implementation kinda provides the low-level requirements for pathsend to work, but this is definitely missing:

  • tests
  • user-facing implementations (that should relay on the current work) like
    • app.serve_files
    • blacksheep.server.responses.file

I'm not sure I'll have time to work on this in the near future, but I can get back to you as soon as I have some time.

To add a bit of context, essentially this should produce a different set of ASGI response messages for files coming from an existing path when the ASGI scope mention the extension availability.

@RobertoPrevato
Copy link
Member

Hi @gi0baro
No worries, and thank you again for your help.

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.

Add support for ASGI pathsend extension

2 participants