Skip to content

Django ORM "Build an app" tutorial + Code #6359

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

Merged
merged 3 commits into from
Jan 23, 2020
Merged

Django ORM "Build an app" tutorial + Code #6359

merged 3 commits into from
Jan 23, 2020

Conversation

ericharmeling
Copy link
Contributor

No description provided.

example code
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@ericharmeling
Copy link
Contributor Author

@Amruta-Ranade

The Django ORM support announcement has been pushed out a week already, due to lack of docs. I'm going to merge this PR this afternoon, to avoid blocking the announcement again. Hopefully you can get to it by then, but if not, we can always do a post-publish review.

Thanks! :)

Copy link
Contributor

@Amruta-Ranade Amruta-Ranade left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall, the tutorial LGTM. Running into some dev env issues on my laptop for step 4. Giving partial feedback to unblock the PR. Will report back as soon as I get to the end of the tutorial.

Update: The tutorial worked well except for the 404 error on running python3 manage.py runserver 0.0.0.0:8000


This creates a new project in a directory called `myproject`.

Open `myproject/settings.py` and change `DATABASES` to the following:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is confusing since the django-admin startproject myproject command created the following directory structure:

myproject > myproject > settings.py (and a bunch of other files)

So the path should be myproject/myproject/settings.py

## Step 4. Set up and run the Django app

In the project's top directory, use the [`manage.py` script](https://docs.djangoproject.com/en/3.0/ref/django-admin/) to create [Django migrations](https://docs.djangoproject.com/en/3.0/topics/migrations/) that initialize the database for the application:

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For clarity, add a step:

$ cd myproject


{% include copy-clipboard.html %}
~~~ shell
$ pip install django
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change to pip3 just to be safe (to help users avoid the issues I ran into later in the tutorial).

Copy link
Contributor Author

@ericharmeling ericharmeling left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TFTR @Amruta-Ranade !

I updated a few other things, for clarity.

Reviewed 1 of 7 files at r2.
Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @Amruta-Ranade)


v20.1/build-a-python-app-with-cockroachdb-django.md, line 30 at r2 (raw file):

Previously, Amruta-Ranade (Amruta Ranade) wrote…

Change to pip3 just to be safe (to help users avoid the issues I ran into later in the tutorial).

Changed to use python -m pip install, per our discussion.

I also added a note at the top saying that this page uses Python 3.


v20.1/build-a-python-app-with-cockroachdb-django.md, line 84 at r2 (raw file):

Previously, Amruta-Ranade (Amruta Ranade) wrote…

This is confusing since the django-admin startproject myproject command created the following directory structure:

myproject > myproject > settings.py (and a bunch of other files)

So the path should be myproject/myproject/settings.py

Done.


v20.1/build-a-python-app-with-cockroachdb-django.md, line 164 at r2 (raw file):

Previously, Amruta-Ranade (Amruta Ranade) wrote…

For clarity, add a step:

$ cd myproject

manage.py is under one level of myproject, so cding might put them at the lower-level.

No idea why Django overcomplicates the directory structure like this, but I'll make the manage.py read myproject/manage.py, for clarity.

Copy link
Contributor

@jseldess jseldess left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, after fixes discussed offline.

Next step is to create an insecure version.

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @Amruta-Ranade and @ericharmeling)


v20.1/build-a-python-app-with-cockroachdb-django.md, line 30 at r4 (raw file):

## Step 1. Install Django and the CockroachDB backend for Django

Install Django and the CockroachDB backend for Django:

Should we make "Django" and "Cockroach backend for Django" links to relevant places?

- Fixed syntax error in imports.
- Declared `in_retry` variable as global in `retry_on_exception()` wrapper function.
@ericharmeling ericharmeling merged commit 455fd35 into master Jan 23, 2020
@ericharmeling ericharmeling deleted the django-orm branch January 23, 2020 22:43
@jseldess
Copy link
Contributor

@ericharmeling, can you please create a docs issue to add an insecure version? Also, can we port this to 19.2 and possibly 19.1, or are we dependent on 20.1 for some reason? @awoods187?

@awoods187
Copy link
Contributor

We can support 19.2 and 20.1. There are a number of things that don't work in 19.2 (due to bugs/missing features in CRDB that can't be backported) but they are well documented on the readme. I don't think we can go to 19.1.

@jseldess
Copy link
Contributor

OK, thanks, @awoods187. @ericharmeling, we should also create an issue to enhance the tutorial to call out the bugs/limitations in the readme. And we should probably open yet another issue to enhance it with any Django-specific best practices we know about (like we've done for SQLAlchemy)

@ericharmeling
Copy link
Contributor Author

@jseldess see:
#6366
#6367
#6368

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

Successfully merging this pull request may close these issues.

6 participants