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

Allow serving of GZIP version of content from static source #5

Closed
wants to merge 1 commit into from

Conversation

mitchins
Copy link

For example we might want to include bootstrap or Jquery (mobile even) in our project so we can have a nice UI that truly works offline (or when the internet drops out/unavailable).
On the ESP8266 it is particularly slow to server content and most likely serving 100kb or so will cause it to fail, quite easily the same content can be GZIPPED and the browser will use it happily but for JS/CSS we'll transfer 1/4 of the content making it much faster and less likely to cause ENOMEM.

-Enabled the WebApp to check for .gz with minimal of extra code, using
the try/catch so it doesn’t rely on os.path which may not exist
-Renamed ‘import re’ to ‘import ure as re’ as the example web app
didn’t run locally if you only have ure.

screen shot 2017-01-18 at 3 02 19 pm

…rsion present and accepted by client

-Added example for the gzip server
-Enabled the WebApp to check for .gz with minimal of extra code, using
the try/catch so it doesn’t rely on os.path which may not exist
-Renamed ‘import re’ to ‘import ure as re’ as the example web app
didn’t run locally if you only have ure.
@pfalcon
Copy link
Owner

pfalcon commented Jan 20, 2017

Thanks for the patch. I agree that it would be useful to support gzip Content-Encoding, but I'm not sure I agree that it's done with "minimal of extra code", nor I'm sure about the overall approach (e.g. compare with https://en.wikipedia.org/wiki/Convention_over_configuration, often employed in web microframeworks). So, I guess this needs further consideration.

-Renamed ‘import re’ to ‘import ure as re’ as the example web app didn’t run locally if you only have ure.

Thanks. That deserved a separate commit, as it's not related to "Added the ability to serve pre-compressed gzip". I handled that now, please rebase.

On the ESP8266 ...

And that's the main question - did you actually tried picoweb on esp8266? Even uasyncio, required for picoweb, isn't yet support on esp8266 (only uasyncio.core is). But if you tried and something of it worked, that's great news, please share the details (e.g. link if it's described elsewhere).

@mitchins
Copy link
Author

HI PFalcon, thanks for the feedback. It does work, please have a look below screenshot.

Is it because of this pull request 19 days ago?
micropython/micropython-lib#141

I'm not sure why it works, but I'm happy it does, the template renders now, and with GZIP it can pull JQuery. I'll rebase it, we can discuss the convention. what suggestions would you propose?

esp8266_pico

@pfalcon
Copy link
Owner

pfalcon commented Jan 29, 2017

@mitchins : Thanks for the screenshot, lovely! So, seeing such community push, I went thru the code uasyncio code to fix issues which may cause problems with esp8266 (because it's nice to see it working, but I know it might break) - that's ahead of planned full optimization work which will require more time. So, uasyncio is now soft-launched for esp8266: http://forum.micropython.org/viewtopic.php?f=16&t=2966 . Feel free to share your further experiences on that thread.

As for subject of this patch, I'd like first to finish planned optimizations, then do careful profiling, and only then start adding more features, carefully profiled still. I'm pretty busy now, so that may take several weeks or few months. Of course, use it as is in your applications now.

@pfalcon
Copy link
Owner

pfalcon commented Jan 30, 2018

https://github.com/pfalcon/picoweb/blob/master/examples/example_extra_headers.py shows what was standardized upon in the mainline.

Thanks for bringing up the issue and working towards a solution!

@pfalcon pfalcon closed this Jan 30, 2018
@mitchins
Copy link
Author

Awesome, thanks!

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