DjangoGirls / tutorial

This is a tutorial we are using for Django Girls workshops
http://tutorial.djangogirls.org/
Other
1.53k stars 1.86k forks source link

Add import os. #1720

Closed Brzozova closed 3 years ago

Brzozova commented 3 years ago

Without importing os module, database creation with python manage.py migrate command will crush with error:

   'NAME': os.path.join(BASE_DIR, 'db.sqlite3'),
   NameError: name 'os' is not defined
das-g commented 3 years ago

Hmm ... for me (Python 3.8.9, Django 2.2.24) the mysite/settings.py file generated by django-admin startproject mysite . already has import os.

It looks like this (click to expand) ```python """ Django settings for mysite project. Generated by 'django-admin startproject' using Django 2.2.24. For more information on this file, see https://docs.djangoproject.com/en/2.2/topics/settings/ For the full list of settings and their values, see https://docs.djangoproject.com/en/2.2/ref/settings/ """ import os # Build paths inside the project like this: os.path.join(BASE_DIR, ...) BASE_DIR = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) # Quick-start development settings - unsuitable for production # See https://docs.djangoproject.com/en/2.2/howto/deployment/checklist/ # SECURITY WARNING: keep the secret key used in production secret! SECRET_KEY = 's56)4$zt8s84fn)8n5y&ewidy5h%7%n4c$i=5!l^m*yl9crt@*' # SECURITY WARNING: don't run with debug turned on in production! DEBUG = True ALLOWED_HOSTS = [] # Application definition INSTALLED_APPS = [ 'django.contrib.admin', 'django.contrib.auth', 'django.contrib.contenttypes', 'django.contrib.sessions', 'django.contrib.messages', 'django.contrib.staticfiles', ] MIDDLEWARE = [ 'django.middleware.security.SecurityMiddleware', 'django.contrib.sessions.middleware.SessionMiddleware', 'django.middleware.common.CommonMiddleware', 'django.middleware.csrf.CsrfViewMiddleware', 'django.contrib.auth.middleware.AuthenticationMiddleware', 'django.contrib.messages.middleware.MessageMiddleware', 'django.middleware.clickjacking.XFrameOptionsMiddleware', ] ROOT_URLCONF = 'mysite.urls' TEMPLATES = [ { 'BACKEND': 'django.template.backends.django.DjangoTemplates', 'DIRS': [], 'APP_DIRS': True, 'OPTIONS': { 'context_processors': [ 'django.template.context_processors.debug', 'django.template.context_processors.request', 'django.contrib.auth.context_processors.auth', 'django.contrib.messages.context_processors.messages', ], }, }, ] WSGI_APPLICATION = 'mysite.wsgi.application' # Database # https://docs.djangoproject.com/en/2.2/ref/settings/#databases DATABASES = { 'default': { 'ENGINE': 'django.db.backends.sqlite3', 'NAME': os.path.join(BASE_DIR, 'db.sqlite3'), } } # Password validation # https://docs.djangoproject.com/en/2.2/ref/settings/#auth-password-validators AUTH_PASSWORD_VALIDATORS = [ { 'NAME': 'django.contrib.auth.password_validation.UserAttributeSimilarityValidator', }, { 'NAME': 'django.contrib.auth.password_validation.MinimumLengthValidator', }, { 'NAME': 'django.contrib.auth.password_validation.CommonPasswordValidator', }, { 'NAME': 'django.contrib.auth.password_validation.NumericPasswordValidator', }, ] # Internationalization # https://docs.djangoproject.com/en/2.2/topics/i18n/ LANGUAGE_CODE = 'en-us' TIME_ZONE = 'UTC' USE_I18N = True USE_L10N = True USE_TZ = True # Static files (CSS, JavaScript, Images) # https://docs.djangoproject.com/en/2.2/howto/static-files/ STATIC_URL = '/static/' ```

Are you maybe using a different Version of Django, @Brzozova?

Brzozova commented 3 years ago

Yes, I use Django==3.1 and Python==3.8.10. Good catch.

Brzozova commented 3 years ago

Sooo I googled a little bit and found this solution: https://stackoverflow.com/questions/63436120/django-new-version-3-1-the-settings-file-have-some-changes

It tells it's not always required to import os module, but you can also change form of STATIC_ROOT.

Changing this: STATIC_ROOT = os.path.join(BASE_DIR, 'static')

To this: STATIC_ROOT = BASE_DIR / 'static'

Also, I found that import os is not by default for Django 3.1+ versions: https://docs.djangoproject.com/en/3.2/releases/3.1/#miscellaneous

Quote: "The settings.py generated by the startproject command now uses pathlib.Path instead of os.path for building filesystem paths."

What do you think about it? Should I add some kind of warning, that "if you use Django 3.1+ versions, remember to import os"?

das-g commented 3 years ago

What do you think about it? Should I add some kind of warning, that "if you use Django 3.1+ versions, remember to import os"?

The proper long-term solution is to upgrade the tutorial to Django 3.x, with all that entails. Some work towards that goal has been begun on branch django-3.1, mainly by @nikhiljohn10. Help there would be appreciated (both reviewing / testing, as well as completing the still-missing steps if any.)

nikhiljohn10 commented 3 years ago

What do you think about it? Should I add some kind of warning, that "if you use Django 3.1+ versions, remember to import os"?

The proper long-term solution is to upgrade the tutorial to Django 3.x, with all that entails. Some work towards that goal has been begun on branch django-3.1, mainly by @nikhiljohn10. Help there would be appreciated (both reviewing / testing, as well as completing the still-missing steps if any.)

Currently tutorial is using 2.2.4. In the migration to Django 3.1, this issue is resolved by adopting the new method from the pathlib module. This is the default for versions from 3.1 of Django. So once the updated files are reflected on the website, everything is going to be fine.

For now, people need to use the requirement.txt file with Django~=2.2.4 as directed in the tutorial.

Brzozova commented 3 years ago

Great, I already finished a blog with Django 3.1 using DjangoGirls tutorial and it was the only problem I found during this process. Also, I am eager to help with documentation development for Django 3.1.

To sum up, if you already created separate documentation for Django 3.1 and implemented pathlib module to fix it, then I suppose this PR can be closed.

nikhiljohn10 commented 3 years ago

Great, I already finished a blog with Django 3.1 using DjangoGirls tutorial and it was the only problem I found during this process. Also, I am eager to help with documentation development for Django 3.1.

You can find the migration PR at #1679 to contribute. There is an extended version of Django tutorial where many changes are not reflected I suppose. So all these bugs and enhancements are to be tracked and fixed.

To sum up, if you already created separate documentation for Django 3.1 and implemented pathlib module to fix it, then I suppose this PR can be closed.

pathlib module is adopted by Django since version 3.1. So I suppose you can close this PR as it is clear that importing os module will not be a suggested way to refer paths in future.