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

Set up the groundwork for a port to Diesel #589

Merged
merged 10 commits into from Mar 7, 2017

Commits on Mar 7, 2017

  1. Set up the groundwork for a port to Diesel

    This PR changes the `users#show` endpoint to use Diesel instead of
    rust-postgres, (and adds a test for that endpoint). I chose that
    endpoint as it seemed likely to touch as few things as possible, but
    enough to require setting up Diesel connection pooling, and testing.
    
    I have also ported over the migrations to use Diesel's migration
    infrastructure. The migration files (except the final migration) were
    generated programatically. Any non-reversible migrations were not dumped
    as these contained procedural data updates which aren't relevant to
    setting up a new database.
    
    `cargo run --bin migrate` will move all the entries in the old
    migrations table into the one used by Diesel. That function is brittle
    since it relies on Diesel internals. However, it really only matters for
    existing contributors (and one deploy to Heroku), so when that function
    breaks we can just delete it.
    
    I've added an additional migration to make the schema compatible with
    `infer_schema!`, which doesn't support tables that have no primary key.
    I'm not using `infer_schema!` just yet, as it doesn't work with non-core
    types and there are some tsvector columns.
    
    I re-ordered the columns on the `User` struct, as diesel emits an
    explicit select clause and then fetches columns by index rather than by
    name. This means that the order has to match how it was defined to
    Diesel (which in the case of `infer_schema!` will be the database
    definition order). If we don't want this restriction, we can replace the
    `infer_schema!` call with manual `table!` calls (which can be
    automatically generated by `diesel print-schema`), and have the columns
    ordered there to match the struct.
    
    Differences to note
    -------------------
    
    The rust-postgres connection is set to require TLS when the env var
    `HEROKU` is set. In Diesel, this would be to add `?sslmode=require` onto
    the database URL. I'm unsure if heroku-postgres URLs already have this
    or not, so I have not added any explicit code for that. It's a question
    that should be answered before this is deployed.
    
    Additionally, I chose not to automatically wrap each request in a
    transaction. It's unneccessary most of the time, and seems like it was
    only done to make testing easier. Since Diesel has explicit support for
    "run tests in a transaction that never commits", we don't need that
    here.
    sgrif committed Mar 7, 2017
    Copy the full SHA
    067eb7c View commit details
    Browse the repository at this point in the history
  2. Attempt to get travis passing

    sgrif committed Mar 7, 2017
    Copy the full SHA
    39c7a18 View commit details
    Browse the repository at this point in the history
  3. Require SSL on Heroku

    sgrif committed Mar 7, 2017
    Copy the full SHA
    034ed98 View commit details
    Browse the repository at this point in the history
  4. Don't rely on a database connection at compile time

    This isn't strictly necessary, but inferring the `crates` table requires
    a feature that Diesel won't have until 0.12, and this has the fewest
    unanswered questions.
    sgrif committed Mar 7, 2017
    Copy the full SHA
    f5f3d85 View commit details
    Browse the repository at this point in the history
  5. Copy the full SHA
    b090a86 View commit details
    Browse the repository at this point in the history
  6. Run Diesel migrations in the Procfile

    I've left the original migration binary in there for now, since we need
    to migrate the database to use Diesel's migration infrastructure on the
    first deploy. After this commit has been deployed it can be removed.
    sgrif committed Mar 7, 2017
    Copy the full SHA
    5082c78 View commit details
    Browse the repository at this point in the history
  7. Add diesel-cli to the buildpacks, don't drop schema_migrations

    We'll want to drop `schema_migrations` sometime soonish, but for now
    let's make sure that everything just works. We don't want to incur
    downtime if we have to roll back.
    sgrif committed Mar 7, 2017
    Copy the full SHA
    525baa3 View commit details
    Browse the repository at this point in the history
  8. Copy the full SHA
    203555f View commit details
    Browse the repository at this point in the history
  9. Decrease the Diesel pool size to 1

    R2D2 tries to immediately populate the pool, and we're running out of
    connections when it does so. Since Diesel is only used on one endpoint,
    we don't need 10 connections.
    sgrif committed Mar 7, 2017
    Copy the full SHA
    c35dd07 View commit details
    Browse the repository at this point in the history
  10. Specify bin

    sgrif committed Mar 7, 2017
    Copy the full SHA
    0cdc64e View commit details
    Browse the repository at this point in the history