Improve settings.py

[Dhruvacube]

Pull Request

What does this PR do?

Fixes #434 and custom issues which I found

What part does this affect?

• FrontEnd.
• BackEnd.
• Documentation.
• Other. (Please specify below)

This PR fixes some security issues related with settings.py also apps was done wrongly , so it also fixes that, also applied some changes so that it is compatible with Django 3.2

NOTE: I am coming from Hacktoberfest :)

Before submitting

• Was this discussed/approved via a GitHub issue or slack?
• Did you read the contributor guideline?
• Did you ensure that there aren't any other open Pull Requests for the same update/change?
• Did you make sure the title is self-explanatory and the description concisely explains the PR?
• Did you make sure your PR does only one thing, instead of bundling different changes together?
• Did you make sure the code is clean and docstrings have been added or updated as required?
• Did you make sure the code is linted/formatted locally prior to submission? (using black and/or prettier)
• Did you make sure to update the documentation with your changes? (if necessary)

PR review

Anyone in the community is free to review the PR once the tests have passed.

Thank you for contributing to AutoDL. We look forward to your continued support.

[Dhruvacube]

@RusherRG its a follow up pr for #438

[ADI10HERO]

Hey, @Dhruvacube most changes I see are unnecessary. Please create an issue first and discuss before making the PR.

[Dhruvacube]

Hey, @Dhruvacube most changes I see are unnecessary. Please create an issue first and discuss before making the PR.

@ADI10HERO ig its necessary changes also its follow-up pr for the #438 also reviewer was there, also read the django docs security docs, security are important.
Since the repo is getting deployed, so security wise you should check , also read this https://docs.djangoproject.com/en/3.2/howto/deployment/checklist/
https://docs.djangoproject.com/en/3.2/ref/settings/#sessions
https://django-secure.readthedocs.io/en/v0.1.1/settings.html
https://docs.djangoproject.com/en/3.2/ref/settings/#secure-browser-xss-filter

The #438 was closed and it was split into #442 and #444 , and ig the reviewer was @RusherRG ||sorry for the ping||

[RusherRG]

@Dhruvacube I think what @ADI10HERO means is that it would be better if you could create an issue first so that we can have a better understanding of what needs to be solved and discuss various solutions before actually opening a PR (having the PRs restricted to resolving one issue at a time). So please do the needful.

[Dhruvacube]

a issue is created #445 :)

[ADI10HERO]

Hey @Dhruvacube , security wise it's fine. But can you please revert those autopep8 changes? And make really small and concise PRs?

Ideally security PR should be separate with no change but the sec fixes.
"Fixed a typo + custom fixes I found" isn't the right way to go about it right?

Apart from that, if it's something as imp as a security thing, I'd highly recommend having a discussion on slack or email (see Security.md)

[Dhruvacube]

Hey @Dhruvacube , security wise it's fine. But can you please revert those autopep8 changes? And make really small and concise PRs?

Ideally security PR should be separate with no change but the sec fixes. "Fixed a typo + custom fixes I found" isn't the right way to go about it right?

Apart from that, if it's something as imp as a security thing, I'd highly recommend having a discussion on slack or email (see Security.md)

okay, yess it something important in the lines of security!!! ohkay I didn't do the discussion on slack since I am not very much comfortable with slack. Also I since I didn't your mail so couldn't contact you :)

"Fixed a typo + custom fixes I found" ig maybe wasn't the right right approach , since I found those I thought its better to make a pr then have discussion:) well nvm.

and for that autopep8 chances I might not be able to revert those but I can format those with blck since someone told me that you guys are using black for formatting :) @ADI10HERO

[Dhruvacube]

@ADI10HERO do ping me if it is okay to format with black , then I will update the pr accordingly :)

[Dhruvacube]

if formatting with blavk is not feasible then could you please how should I revert those changes?

[ADI10HERO]

Formatting with black should be fine!

However, I suggest you rebase the PR to the main of the repo and just make 1 change per PR!
Both your PRs are messy and tbh, no maintainer would merge a messy PR that doesn't clearly say what it does. I am not trying to be rude here...

If I were you, I'd have rebased this PR (close the other one) and discuss the issue on either GitHub issues or slack, propose a solution, get feedback and then make the PR. (at least when it comes to improving settings.py)

I also wouldn't have touched any file that shouldn't have been changed in that particular PR.
For eg., Your logging PR changes some files (apparently due to linter) which have nothing to do with the changes you're proposing.

[ADI10HERO]

Also, please read contributing guidelines and make appropriate changes.

Thanks and BR!

[Dhruvacube]

Also, please read contributing guidelines and make appropriate changes.

Thanks and BR!

okay sure, just lemme get some time, then for sure, I will make the necessary changes

[ADI10HERO]

Hey closing this PR due to inactivity! (Please check out the pinned announcement)