search

pylti / lti

Learning Tools Interoperability for Python
Other
78 stars 45 forks source link

Add Django middleware and backend #48

Closed kris-steinhoff closed 6 years ago

kris-steinhoff commented 7 years ago

I've been using this code in production for a while, and it's been working well.

Please let me know if there's any other context or information I can provide.

codecov[bot] commented 7 years ago

Codecov Report

Merging #48 into master will decrease coverage by 32.65%. The diff coverage is 0%.

Impacted file tree graph

@@             Coverage Diff             @@
##           master      #48       +/-   ##
===========================================
- Coverage   83.64%   50.98%   -32.66%     
===========================================
  Files          16       23        +7     
  Lines         648     1063      +415     
  Branches      113      135       +22     
===========================================
  Hits          542      542               
- Misses         78      493      +415     
  Partials       28       28
Impacted Files Coverage Δ
src/lti/contrib/django/middleware.py 0% <0%> (ø)
src/lti/contrib/django/utils.py 0% <0%> (ø)
src/lti/contrib/django/tests.py 0% <0%> (ø)
src/lti/contrib/django/migrations/0001_initial.py 0% <0%> (ø)
src/lti/contrib/django/apps.py 0% <0%> (ø)
src/lti/contrib/django/backends.py 0% <0%> (ø)
src/lti/contrib/django/models.py 0% <0%> (ø)
... and 4 more

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 41f5e28...70c154c. Read the comment docs.

kris-steinhoff commented 7 years ago

Thanks for the feedback. I'll work on the changes and submit an update in the next week or so.

kris-steinhoff commented 7 years ago

I still intend to work on this. Other projects have been eating up my time. Sorry for the delay.

kris-steinhoff commented 7 years ago

I've made some of the requested changes.

I'm not sure when I'd be able to get to the more extensive work around the NonceHistory model. I've changed jobs recently and I'm not sure I'll have the bandwidth soon.

ryanhiebert commented 7 years ago

Cool. Thanks for letting us know that you're not likely to look at it right away. That lets someone else pick it up if they want to. Let us know if you're able to pick it up again later.

michaelwheeler commented 6 years ago

I might be interested in helping to wrap this up, if it would be okay with everyone.

kris-steinhoff commented 6 years ago

Thanks, that would be great!

michaelwheeler commented 6 years ago

Great! So it looks like what remains to be done is to get the tests running and passing, and to sort out nonce storage.

How would everyone feel about just storing nonce history using Django's caching framework, since it already provides multiple storage backends? Since each cache backend handles expiration/culling, I think a cleanup command may not be needed.

michaelwheeler commented 6 years ago

I've got some updates ready to push for this, but I'm not sure of the best way to go about it since I don't have access to this branch. Any suggestions?

ryanhiebert commented 6 years ago

Oh, I didn't think about that. I'd suggest making a new fork, and opening a new pull request, making reference to this one.

kris-steinhoff commented 6 years ago

Closing in favor of #66.