search

18F / autoapi

A basic spreadsheet to api engine
Other
42 stars 18 forks source link

Removing psycopg2 from requirements.txt breaks Cloudfoundry deploys #85

Closed toolness closed 7 years ago

toolness commented 8 years ago

psycopg2 was removed from requirements.txt as part of #76, which was in one sense the right thing to do because the product is database agnostic... However, practically speaking, the Cloudfoundry deploy of autoapi uses postgres, so deploying to autoapi-ed.apps.cloud.gov broke when I used the latest master branch. I had to roll back to just before 6a80fbda7e2d0972df915348bdaf42c3a57cefcc in order to deploy successfully.

I'm not sure what to do about this. I'm wary of reverting 6a80fbda7e2d0972df915348bdaf42c3a57cefcc because it is true that the product should be database agnostic, but I'm not sure how else to ensure that the Cloudfoundry deploy can find psycopg2.

In some Heroku-based projects in the past, I've actually had two separate requirements files:

  1. A requirements.minimal.txt file that has the bare minimum requirements needed to run the app.
  2. A requirements.txt file that includes all deployment-specific requirements such as psycopg2, newrelic, and so forth. It also includes a -r requirements.minimal.txt line so that all the minimal dependencies are included too.

Does this kind of solution seem okay @vrajmohan @catherinedevlin or is there some better solution I'm unaware of?

vrajmohan commented 8 years ago

Looks like a valid strategy to me. It's a pity that the buildpack needs it to be named requirements.txt. In other words, I would have preferred your strategy with: requirements.minimal.txt replaced by requirements.txt and requirements.txt replaced by requirements_with_postgres.txt

catherinedevlin commented 8 years ago

Is it database-agnostic? Should it be? I think of the whole point of AutoAPI as being to throw up a no-think API for people who really do not want to get involved in the details. If somebody is thinking, "no, I want to use MySQL because I've got a sweet customized storage engine I want to try"... that's not our target user, right?

vrajmohan commented 8 years ago

Excellent point. All the more reason that we should address #35 - it helps us be consistent with questions like this. It would make sense to be opinionated with PostgreSQL.

toolness commented 8 years ago

Great points @catherinedevlin and @vrajmohan! I am also quite interested in addressing #35 too.

So it sounds like we should be opinionated and revert 6a80fbda7e2d0972df915348bdaf42c3a57cefcc, then?

vrajmohan commented 8 years ago

:+1: with reinstating psycopg2. We should be able to move back to being DB agnostic if we ever decide that we are building a library rather than an application.