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

Static file handler for aiohttp.web #164

Merged
merged 2 commits into from
Oct 28, 2014
Merged

Static file handler for aiohttp.web #164

merged 2 commits into from
Oct 28, 2014

Conversation

mind1m
Copy link
Contributor

@mind1m mind1m commented Oct 27, 2014

No description provided.

@telendt
Copy link

telendt commented Oct 27, 2014

I'd rather prefer to run all file IO operations in ThreadPoolExecutor, so that they don't block the loop. There are also some features missing, like Content-Range/Last-Modified/ETag support.

@fafhrd91
Copy link
Member

i'd rather have static handler as helper class. is it possible to do something like this?

   app.router.add_route('/static/*', StaticFilesHandler(filesystem_path))

@mind1m
Copy link
Contributor Author

mind1m commented Oct 27, 2014

I think, generally, ThreadPoolExecutor makes sense, so does Content-Range/Last-Modified/ETag. However, as far as I understand, this meant to be just the development solution; for production nginx or similar should be used for static handling. @fafhrd91 @asvetlov what do you think?

@fafhrd91
It is possible, but it would rather look like this:

app.router.add_route('GET', r'/static/(?P<filename>.*)', StaticFilesHandler(filesystem_path))

Here method and regex seem to be unnecessary, but this is the only way it could be done without modifying add_route

@fafhrd91
Copy link
Member

ok, add_route solution looks ugly.
in general i am +0 on that.

asyncio does not support async operations on files, so i don't think serving static files is good idea for production.

@asvetlov asvetlov merged commit 6e04d1a into aio-libs:master Oct 28, 2014
@asvetlov
Copy link
Member

I think static file handling is useful feature for development-only.

Merged.
Support for Content-Range and caching is useful in general but can be done in separate pull request (see #165).

@ludovic-gasc
Copy link
Contributor

+1 for this PR, it's necessary during development process.

@lock
Copy link

lock bot commented Oct 30, 2019

This thread has been automatically locked since there has not been
any recent activity after it was closed. Please open a new issue for
related bugs.

If you feel like there's important points made in this discussion,
please include those exceprts into that new issue.

@lock lock bot added the outdated label Oct 30, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Oct 30, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants