Closed danihodovic closed 4 years ago
Thank you, @danihodovic, looks good! I'll check it a bit later as I am very busy these weeks.
@liminspace any chance of a review for this?
Yes, it looks great. I'm too busy now, but I'm going to do it as soon as I have free time.
Any free time lately? :smiley:
It would be great to install the master release directly from pip than installing my own branch from git.
Considering that this feature is opt-in it won't break any existing setups unless someone explicitly decides to use the HTTP server option.
Hi @danihodovic , your server returns 413 error if I try to send a big query. In unit-tests I use request body with size 8 MB to check that server works correctly. Please, fix your library and I will support it. As variant you can add option --maxbody
to set any size in bytes.
@danihodovic also your server uses twice quoting of result if json {"mjml": "..."} is sent. I can't release supporting your library until it's fixed. more details here https://travis-ci.org/liminspace/django-mjml/builds/623212728
You seem to have mjml compilation problems as far as I can see: https://travis-ci.org/liminspace/django-mjml/jobs/623212729#L271
Also regarding your changes in https://github.com/liminspace/django-mjml/tree/httpserver:
I don't think the client app should have logic for proxying to multiple HTTP servers. Reverse proxies such as Nginx or Apache are best suited to do this, not the Django application.
@danihodovic note, that I tested everything by using MJML Public API and all tests works correctly. try to post json {"mjml": "some mjml with unicode"} and you get result with twice quoting value. that is a problem. if you post just "mjml", it works correctly.
it returns correct value:
response = requests.post(
url=server_conf['URL'],
auth=auth,
data=force_bytes(mjml_code),
timeout=25,
)
it returns double quoting value:
response = requests.post(
url=server_conf['URL'],
auth=auth,
data=force_bytes(json.dumps({'mjml': mjml_code})),
headers={'Content-Type': 'application/json'},
timeout=25,
)
I need to support second variant as it's described in public API which must be supported at first: https://mjml.io/api/documentation/
multiple HTTP-servers are ok, as there can be a lot of servers to render in parallel (in different processes). if you don't need it, just set single one, I hope it's not a problem for you :)
Seems like the public API has changed and a json document is now passed as the payload.
Both interfaces should be supported with https://github.com/danihodovic/mjml-server/pull/56/files
@danihodovic now it works very well, thanks! when are you going to release new version of mjml-http-server
? I need it released to run tests on travis and update the documentation.
0.0.3 has been released https://www.npmjs.com/package/mjml-http-server
the backend for HTTP servers will be released under v 0.9.0
The MJML team has released a HTTP API for integration with non NodeJS apps at https://mjml.io/api.
I've built a self hosted alternative which should be compatible with the API the MJML team has built (without baked in auth). https://github.com/danihodovic/mjml-http-server
It should work with both auth and non-auth, although I have not tested the auth version since https://mjml.io/api is currently invite only.