Skip to content
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

Do we have a specific, immediate need for the /admin endpoint? #15

Open
MikeTheCanuck opened this issue Feb 19, 2017 · 13 comments
Open

Comments

@MikeTheCanuck
Copy link
Collaborator

Currently when running the budget_proj/app without a migrate step, the following message shows up after runserver:

python3 budget_proj/manage.py runserver
Performing system checks...

System check identified no issues (0 silenced).

You have 13 unapplied migration(s). Your project may not work properly until you apply the migrations for app(s): admin, auth, contenttypes, sessions.
Run 'python manage.py migrate' to apply them.

February 19, 2017 - 21:41:45
Django version 1.10.5, using settings 'budget_proj.settings'
Starting development server at http://127.0.0.1:8000/
Quit the server with CONTROL-C.
Not Found: /

The message of note is:

You have 13 unapplied migrations...

I see various projects do or do not include a step like this before runserver - don't know if that's required here:

python3 budget_proj/manage.py migrate
@jimtyhurst
Copy link
Contributor

First need to create a migration:
python3 manage.py makemigrations budget_app
Then apply the migration:
python3 manage.py sqlmigrate budget_app 0001

@hassanshamim
Copy link
Collaborator

Those 13 initial migrations come standard with Django and are for the django admin I believe.

@MikeTheCanuck
Copy link
Collaborator Author

@hassanshamim Any insight into whether those are actually necessary to stand up a Django app, or are they just there to provide some additional functionality that not all Django projects will ever need (e.g. like the NORTHWINDS database that ships with Microsoft SQL Server, which is helpful for newbies but unnecessary once you start writing your own projects)?

@hassanshamim
Copy link
Collaborator

It should run. I accidentally already ran the migrations locally but I think if you don't run them, then http://127.0.0.1:8000/admin/ won't be available. Your other endpoints should be fine?

@MikeTheCanuck
Copy link
Collaborator Author

Does anyone have a specific need for us to implement the /admin endpoint?

If not, I say we leave this migration step of the automated build - reducing surface area means one less piece of code to maintain.

@jimtyhurst
Copy link
Contributor

Migration is required for the database schema to match the model definitions. If you do not do the migration, the web application will not run correctly.

@MikeTheCanuck
Copy link
Collaborator Author

@jimtyhurst are you saying that in general all migrations must be performed to ensure the web app runs correctly (e.g. for each endpoint, run its migration before the endpoint will work), or are you specifically saying that the /admin migrations must be performed or else the rest of the Django APIs won't run correctly?

If the former, then I think I'm hearing that no one currently has a use for the /admin endpoint so the default migrations (13 unapplied that I noticed above) do not need to be applied.

Absolutely, all other endpoints we build will require their associated migrations be applied.

@hassanshamim
Copy link
Collaborator

@MikeTheCanuck - a quick overview: migrations in Django are like git (a version control system) for your database schema. As we add new models or update models we want a way to track those changes, should two developers make such changes on two different branches simultaneously, we then have a way to negotiate those changes. This is especially important in production where you have existing data in the database and you don't want to lose it all just because you changed the type of column in a table.

When we 'run migrations', what that does is read a list of changes in the migrations folder and apply any of those to the current database that haven't been applied yet.

What this means is in development mode, running our migrations only affects the local database (django defaults to using a sqlite3 database). In production or staging when we run our migrations, it will affect those databases. If you try and run the django server and it detects unapplied migrations it will scold you.

I see no reason for migrations not to always be applied. It's best practice, and the worst outcome in my mind if not followed would be that people generate migrations locally but never run them and a situation occurs in production where there's an error with the db or migrations we didn't catch because they weren't ran locally.

@MikeTheCanuck MikeTheCanuck changed the title Does current code require migrate step as well? Do we have a specific, immediate need for the /admin endpoint? Feb 21, 2017
@MikeTheCanuck
Copy link
Collaborator Author

Thanks for the background on the migrate function and what purpose it serves, @hassanshamim - this is very helpful to understand what it does and doesn't do.

OK folks, I've changed the title of this issue because of all the confusion I'm seeing in the discussion. My original question (poorly framed I can see now, thanks for helping me clarify) is simply whether we need to implement the /admin endpoint at present - not in any way 'threatening' the obviously-necessary migrations that are being built up in the /budget_proj/budget_app/migrations/ folder).

Given that I've heard no use case asserted for the /admin endpoint, I recommend we skip past that particular migration in the build of the API container. Least surface area to manage and all that. All other migrations will be of course performed as the database comes online and future database design alterations are made.

Final question: what commands do we need to run to skip the 13 default migrations I originally noted (i.e. the ones associated with the /admin endpoint), and perform the migrations for all in-app functionality (e.g. the OCRB and KPM models being migrated in 0001... and 0002... scripts)?

@hassanshamim
Copy link
Collaborator

I feel like avoiding the django admin (aka default migrations) is more work than what we get out of it. What is the impetus for this? It comes stock with django so it's more secure and dependable than any code we'll be writing.

I think removing django admin from the project settings may work, I just don't see the advantage.

@MikeTheCanuck
Copy link
Collaborator Author

Impetus at first was to figure out if I needed to worry about the "13 migrations". I hadn't delved into Django to this point, and other examples and tutorials I'd seen were inconsistent on whether you needed to run python manage.py migrate before python manage.py runserver.

As this discussion progressed, I didn't see anyone clamouring for the functionality embedded in the Django default apps (admin, auth, contenttypes, sessions). That finally caught my attention, and my experience over the years has taught me that if one doesn't have a need for specific functionality, it's advisable not to include it. If one later finds they need it, then it can always be added back later.

My inclinations as a systems guy is to implement the least functionality necessary to fulfil the needed purposes of the system. As a security guy I'm inclined to have as few entry points in the system as we need, to keep us from forgetting about back doors and weak points (and not having to be quite as vigilant about patching stuff same day). I realize those inclinations conflict with the development approach of having as many tools available in case we have some need for them in the future.

At the moment we don't need to resolve this question - until we have a permanent database, we can ignore the "unapplied migrations" warning. I'll keep looking into this to see if my concerns can be allayed.

@MikeTheCanuck
Copy link
Collaborator Author

Notes to self:

  • researched Djangoproject documentation and Mozilla's developer network docs, and didn't find any sign there of how to avoid running the default migrations when running the migrate command - maybe no one has ever wanted to?
  • ran the current migration set and these are what the output coughed up:
$ python3 budget_proj/manage.py migrate
Operations to perform:
  Apply all migrations: admin, auth, budget_app, contenttypes, sessions
Running migrations:
  Applying contenttypes.0001_initial... OK
  Applying auth.0001_initial... OK
  Applying admin.0001_initial... OK
  Applying admin.0002_logentry_remove_auto_add... OK
  Applying contenttypes.0002_remove_content_type_name... OK
  Applying auth.0002_alter_permission_name_max_length... OK
  Applying auth.0003_alter_user_email_max_length... OK
  Applying auth.0004_alter_user_username_opts... OK
  Applying auth.0005_alter_user_last_login_null... OK
  Applying auth.0006_require_contenttypes_0002... OK
  Applying auth.0007_alter_validators_add_error_messages... OK
  Applying auth.0008_alter_user_username_max_length... OK
  Applying budget_app.0001_initial... OK
  Applying budget_app.0002_auto_20170221_0359... OK
  Applying sessions.0001_initial... OK

@mxmoss
Copy link
Contributor

mxmoss commented Apr 29, 2017

@MikeTheCanuck I moved this into "in progress" because it seems like you're working on this with PR #142

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

No branches or pull requests

4 participants