Skip to content

Remove unnecessary content about static files for tutorial-django #6161

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

Conversation

imba-tjd
Copy link
Contributor

@imba-tjd imba-tjd commented Mar 24, 2023

There is no need to mention ALLOWED_HOSTS at this point.

For staticfiles_urlpatterns and Issue 13, see microsoft/python-sample-vscode-django-tutorial#13 (comment)
If you use django's runserver, you don't need them. If you set DEBUG=False, they take no effect anyway.

With that, most of them become unnecessary, even though When switching to production, navigate to settings.py, set DEBUG=False is correct.

@luabud
Copy link
Member

luabud commented Mar 24, 2023

Thanks for your suggestion! After talking to my colleague who has a ton of Django experience, we decided to leave this section in our docs giving it's not technically incorrect and provides info that could be helpful -- for example, it leads into more explanation about how allowed hosts works in production. ALLOWED_HOSTS is a field that is pre-populated by django-management command in a brand new project, and it needs to be altered in order to use it in production.

If you would be open to help us improve the paragraph by explaining what ALLOWED_HOSTS are (for example with the paragraph below), we'd be more than happy to take a contribution!
Otherwise we can do that as a separate PR 😊

..., and change ALLOWED_HOSTS = ['*'], which uses a wildcard to allow any host, to a comma-separated list of specific hosts for a more secure deployment. 

@imba-tjd
Copy link
Contributor Author

imba-tjd commented Mar 25, 2023

🆗leave ALLOWED_HOSTS as is.
How about staticfiles_urlpatterns()? Like I said, if you use python manage.py runserver, you don't need this. And if you set DEBUG=False, you also don't need this.

@luabud
Copy link
Member

luabud commented Mar 28, 2023

Even though it may not be needed, I'm not convinced it's not helpful to leave it there for additional scenarios. @dawnwages -- would appreciate your guidance/opinions here!

@dawnwages
Copy link

dawnwages commented Apr 5, 2023

Agreed with @luabud

re: https://code.visualstudio.com/docs/python/tutorial-django#_serve-static-files

When switching to production, navigate to settings.py, set DEBUG=False, and change ALLOWED_HOSTS = ['*'] to allow specific hosts. This may result in additional work when using containers. For details, see Issue 13.
 

I would say keep it. This is the only point in the tutorial that ALLOWED_HOSTS is mentioned. It is not technically incorrect. It leads into more explanation about how allowed hosts works in production and ALLOWED_HOSTS is a field that is pre-populated by django-management command in a brand new project.

@gregvanl gregvanl closed this Apr 5, 2023
@gregvanl
Copy link

gregvanl commented Apr 5, 2023

Closing this PR as the extension team would like to keep the documentation as is.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants