add handling of unhandledrejection #12
Conversation
|
Sorry, I ran the demo server and realized it wasn't working any more. So I made the changes required to work with Django 1.10, 1.11 and 2.0. I guess we also need a demo of the promise based error handling? |
demo/MANIFEST.in
Outdated
| @@ -1,3 +0,0 @@ | |||
| recursive-include demoproject/templates * | |||
There was a problem hiding this comment.
Not needed. Maybe I misunderstood what the contents of that folder were for, but it didn't work to run it using the included the instructions, and it does run now.
|
I think you should put your last commit in a new PR |
README.rst
Outdated
|
|
||
| $ git clone git://github.com/jojax/django-js-error-hook.git | ||
| $ cd django-js-error-hook | ||
| $ virtualenv env | ||
| $ env/bin/python setup.py develop | ||
| $ python3 -m venv env |
There was a problem hiding this comment.
I am not a fan on mixing two versions of Python here.
There was a problem hiding this comment.
It is not mixing different versions. It is using Python 3 throughout.
This needs to be done because the way you have setup the setup.py file, it automatically installs django 2.0 -- which only runs on python 3.
There was a problem hiding this comment.
Actually it depends if something else is already installed in the venv when you install the demo. You can specify a different version of Python and Django to run the demo with but I agree with you that this enforce Python 3 somehow while this lib is still compatible with both.
But we can agree to only support Python 3.6+ from now on.
There was a problem hiding this comment.
The instructions make the user create a new venv, so there cannot be anything installed in that venv already. I am only switching that venv from being a Python 2 to a Python 3 venv. This is required because the system subsequently tries to install Django 2.0 into the venv -- which won't run in a Python 2 venv.
README.rst
Outdated
| $ cd demo | ||
| $ ../env/bin/python demo/setup.py develop |
There was a problem hiding this comment.
I'd lilke to keep the demo installable in the venv.
There was a problem hiding this comment.
It is installable in the venv.
There was a problem hiding this comment.
The line source env/bin/activate means that all the subsequent python command calls are using the python from the virtual env.
There was a problem hiding this comment.
My point is that a lot of changes that you've made here are not about porting to Django 2.0 but are removing features that were implemented in order for people to install the demo in a venv and run it without having to move in the demo folder.
There was a problem hiding this comment.
What features?
There was a problem hiding this comment.
It is all still running in the venv. The venv is being activated in line 10. The line "cd" is still moving the user into the demo folder, like before.
There was a problem hiding this comment.
I agree with you that the dependencies are installed in the venv the demoproject however is not installed in the venv as a python project anymore (no more setup.py that keeps the list of requirements etc) I guess it is not really a big deal since nobody is supposed to need to package it.
There was a problem hiding this comment.
Ok. yeah, I may have misunderstood what the purpose with some of this was. I received a lot of error messages and the instructions gave me instructions that didn't quite work, so I tried to update as good as I could. If there is anything missing now that used to be there, the easiest is probably if you change it to work again.
|
OK, I have added the demo/test for the unhandled Promise. The upgrade to work with Django 1.10, 1.11 and 2.0 was needed because your setup.py automatically installs django 2.0 for the demo, so there was no other way of getting the demo project to run. You are welcome to change things around and turn it into several PRs. |
There was a problem hiding this comment.
I just now discovered there is also travis running. So I made some changes for that as well. I have only tested this on Django 2.0. Everything should also run on 1.10/1.11 except maybe the manage.py files. The old ones did not work any more, so I took new ones from Django 2.0. I don't know if they work with lower django versions as well.
README.rst
Outdated
| $ cd demo | ||
| $ ../env/bin/python demo/setup.py develop |
There was a problem hiding this comment.
What features?
README.rst
Outdated
| $ cd demo | ||
| $ ../env/bin/python demo/setup.py develop |
There was a problem hiding this comment.
It is all still running in the venv. The venv is being activated in line 10. The line "cd" is still moving the user into the demo folder, like before.
README.rst
Outdated
|
|
||
| $ git clone git://github.com/jojax/django-js-error-hook.git | ||
| $ cd django-js-error-hook | ||
| $ virtualenv env | ||
| $ env/bin/python setup.py develop | ||
| $ python3 -m venv env |
There was a problem hiding this comment.
The instructions make the user create a new venv, so there cannot be anything installed in that venv already. I am only switching that venv from being a Python 2 to a Python 3 venv. This is required because the system subsequently tries to install Django 2.0 into the venv -- which won't run in a Python 2 venv.
|
While testing this I've seen the following error in the console: I guess we need to add a CSRF exempt decorator to the error_hook endpoint. |
|
Merged with 7e67c92 |
|
:) ok. I didn't get that error. But maybe it depends on the python or django version. |
|
@johanneswilm Thank you for your work on that. |
fixes #11