Black

[stephane]

What do you think about using Black?

[codecov[bot]]

Codecov Report

Merging #256 (161e417) into master (de92fa1) will decrease coverage by 0.1%. The diff coverage is 78.3%.

@@           Coverage Diff            @@
##           master    #256     +/-   ##
========================================
- Coverage    85.4%   85.3%   -0.2%     
========================================
  Files          11       8      -3     
  Lines         556     504     -52     
========================================
- Hits          475     430     -45     
+ Misses         81      74      -7     

Impacted Files	Coverage Δ
django_auth_adfs/drf-urls.py	0.0% <ø> (ø)
django_auth_adfs/drf_urls.py	100.0% <ø> (ø)
django_auth_adfs/exceptions.py	100.0% <ø> (ø)
django_auth_adfs/rest_framework.py	70.8% <57.1%> (ø)
django_auth_adfs/middleware.py	90.9% <75.0%> (ø)
django_auth_adfs/config.py	88.0% <78.5%> (ø)
django_auth_adfs/backend.py	84.0% <82.8%> (ø)
django_auth_adfs/urls.py
django_auth_adfs/views.py

[JonasKs]

Hi, we’ve discussed this previously, and there’s a PR for it in #178. We decided to keep git history back then, since this library isn’t initially developed by me or Sondre, but transferred from Jobec. We added a bunch of features and maintained after transfer, but I will respect his wish.

Running black removes all git blame, for not much gain. This library is at a maintenance stage, and not really actively developed(Azure specifically is in need of a big overhaul). If this changes, I think we’ll reconsider. Until then, I’m afraid I’ll deny this based on the previous discussion.

[stephane]

OK, thank you for your feedback.

[tim-schilling]

@JonasKs The concern about removing git blame is significantly reduced by.git-blame-ignore-revs. I think it's worth discussing centered around that since the original discussion mentioned it at the end, but there wasn't a rebuttal.

[stephane]

This library is at a maintenance stage, and not really actively developed(Azure specifically is in need of a big overhaul). If this changes, I think we’ll reconsider. Until then, I’m afraid I’ll deny this based on the previous discussion.

I'm a bit worried about this comment because I'm in the process to use this library in apps that require long term support. Should I use a library based on https://github.com/AzureAD/microsoft-authentication-library-for-python instead? Eg. https://github.com/AgileTek/django-azure-auth

[tim-schilling]

Maintenance doesn't mean a lack of support. I don't think any of the snok developers are going anywhere. However, it does mean new feature development is going to rely on the broader community to be the impetus.

[JonasKs]

^ this. Sorry if that was confusing.

Security is my #1 priority. I've developed FastAPI-azure-auth, and I(almong others @snok) will continue to support and maintain this library. How ever, I'm no longer actively using Django, so I will personally not expand its features 😊

If someone decides to really work on its features/azure documentation and use cases, I will happily review and help.

I can't really comment on the other libraries - one seem pretty unused and the official Azure one is very limited and not well integrated with Django and it's ORM(atleast wasn't before).

You're in safe hands here - as you can see we are actively responding and reviewing MRs. I will probably look into making the middleware async (or help/review), and with that maybe Black can be implemented - it'll be a pretty big rewrite.

[stephane]

Oh funny, I didn't know you work with snok, I was using snok/install-poetryfor CI until recently (nowpipx install poetry`). On FastAPI side, I coded an app with FastAPI/SQLAlchemy last year but I've rewritten it in Django + django-ninja (pydantic) in March after serious concerns about concurrency and quality of support. I don't regret my choice BTW.

[JonasKs]

Snok is just an org with friends, collaborating on open source 😊

I understand, however the most important parts of FastAPI is pydantic and starlette, two projects I have full faith in. FastAPI is just a wrapper on top.