Skip to content
This repository was archived by the owner on May 26, 2020. It is now read-only.

[RDY] Added support for Custom Exception Handler - #374

Open
ideallical wants to merge 2 commits into
jpadilla:masterfrom
ideallical:364-enable-custom-exception-handler
Open

ideallical wants to merge 2 commits into
jpadilla:masterfrom
ideallical:364-enable-custom-exception-handler

Conversation

@ideallical

Copy link
Copy Markdown

This PR makes it possible to enable DRF's custom exception handler, by default the old behaviour is kept for backwards compatibility. Typo has been fixed.

@ideallical ideallical changed the title Added support for Custom Exception Handler #364 Added support for Custom Exception Handler Sep 16, 2017
@blueyed

blueyed commented Sep 22, 2017

Copy link
Copy Markdown
Contributor

Please add tests for it.

@ideallical

Copy link
Copy Markdown
Author

Should be ready for merge now, @blueyed

@blueyed

blueyed commented Sep 28, 2017

Copy link
Copy Markdown
Contributor

Thanks!
Can you rebase it please, so we can see how #382 has an impact?

@blueyed blueyed changed the title Added support for Custom Exception Handler [RDY] Added support for Custom Exception Handler Sep 28, 2017
@ideallical
ideallical force-pushed the 364-enable-custom-exception-handler branch from df48485 to 943eb02 Compare September 28, 2017 22:20
@ideallical

ideallical commented Sep 28, 2017

Copy link
Copy Markdown
Author

the branche has been rebased @blueyed
Somehow, the codecov plugin doesn't appear in this thread (although it's being used in Travis). Shall I close this PR and create a new one to see if that helps? @blueyed

@blueyed blueyed closed this Oct 9, 2017
@blueyed blueyed reopened this Oct 9, 2017
@codecov

codecov Bot commented Oct 9, 2017

Copy link
Copy Markdown

Codecov Report

Merging #374 into master will increase coverage by 0.21%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #374      +/-   ##
==========================================
+ Coverage   90.62%   90.84%   +0.21%     
==========================================
  Files          14       14              
  Lines         843      863      +20     
  Branches       29       29              
==========================================
+ Hits          764      784      +20     
  Misses         66       66              
  Partials       13       13
Flag Coverage Δ
#codecov 90.84% <100%> (+0.21%) ⬆️
#dj110 87.71% <100%> (+0.29%) ⬆️
#dj111 87.71% <100%> (+0.29%) ⬆️
#dj18 90.03% <100%> (+0.23%) ⬆️
#dj19 90.03% <100%> (+0.23%) ⬆️
#drf31 90.03% <100%> (+0.23%) ⬆️
#drf32 90.03% <100%> (+0.23%) ⬆️
#drf33 90.03% <100%> (+0.23%) ⬆️
#drf34 90.84% <100%> (+0.21%) ⬆️
#drf35 90.49% <100%> (+0.22%) ⬆️
#drf36 90.49% <100%> (+0.22%) ⬆️
#py27 90.84% <100%> (+0.21%) ⬆️
#py33 89.68% <100%> (+0.24%) ⬆️
#py34 89.68% <100%> (+0.24%) ⬆️
Impacted Files Coverage Δ
rest_framework_jwt/settings.py 100% <ø> (ø) ⬆️
tests/test_views.py 100% <100%> (ø) ⬆️
rest_framework_jwt/views.py 92.5% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 38897e7...943eb02. Read the comment docs.

@blueyed

blueyed commented Oct 9, 2017

Copy link
Copy Markdown
Contributor

Maybe re-opening helps for codecov.

Otherwise I've thought that I've left a comment for this to be the default, but have not?!

@blueyed

blueyed commented Oct 9, 2017

Copy link
Copy Markdown
Contributor

See also #386.

@ideallical

Copy link
Copy Markdown
Author

So, coverage report says coverage went up. Time to merge? @blueyed

@blueyed

blueyed commented Oct 14, 2017

Copy link
Copy Markdown
Contributor

@ideallical
Thanks, but I think this should be just the default, without a setting for this.

@blueyed

blueyed commented Oct 14, 2017

Copy link
Copy Markdown
Contributor

@jpadilla ?

@draskomikic

Copy link
Copy Markdown

Hi guys, I am also interested in this feature, when do you expect it will be merged ?

@ilstreltsov

Copy link
Copy Markdown

@blueyed
Making this behavior (raise exception) as default will break backward compatibility, right? So i suggest just to merge PR as it is. Up!

This branch has not been deployed

No deployments
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants