Making heroku-postgresql addon optional

[BNMetrics]

Use Cases:

• We might not need any databases for some django apps.
• We might use a non-heroku database for a heroku app.

Why is it important:

• We create dev environment heroku apps on demand. It can be confusing to the devs when they see the default heroku DB being created.
• It should work the same as before, the heroku-postgresql addon part will only be included if there is a file stamp no_default_addons in the BUILD_DIR

[edmorley]

Hi! Thank you for the PR :-)

I agree there may be occasions where one might not want the DB to be created. Though I have a few questions about your use case...

We create dev environment heroku apps on demand. It can be confusing to the devs when they see the default heroku DB being created.

What do you mean by "on demand"? Automatic review apps? Or do you mean you create non-heroku DBs by hand?

We might not need any databases for some databases.

(I'm presuming this meant "for some Django apps"?)

The current implementation only creates a DB if there is a Django manage.py, though agree there may be cases where a DB is not required.

I took a look at what the Ruby buildpack does, and it only creates a DB iff using Rails and a postgres adapter gem is installed. Perhaps that's what we should do here instead of having it be opt-out like the current PR implementation?

Also if we did keep the opt-out, I think I prefer an env var - which means we'd need to use an approach similar to Ruby where the release metadata is actually generated during bin/compile (where the ENV_DIR is available) and then loaded back off disk in bin/release.

We'll also need to test this functionality :-)

[edmorley]

Also, for your use-case is the DB still a Postgres DB or something else?

[BNMetrics]

Hi there!

What do you mean by "on demand"? Automatic review apps? Or do you mean you create non-heroku DBs by hand?

We have dev environments that can be created via terraform, with that, we can create environments with various apps on various platforms on demand. We use Non-heroku DBs for some of our applications, e.g. RDS
And yes, it will be postgres, but it shouldn't matter exactly what DB we use outside of heroku 😉

(I'm presuming this meant "for some Django apps"?)

Yes! Typo XD

The current implementation only creates a DB if there is a Django manage.py, though agree there may be cases where a DB is not required.

We also have applications that don't use any DB at all.

I took a look at what the Ruby buildpack does, and it only creates a DB iff using Rails and a postgres adapter gem is installed. Perhaps that's what we should do here instead of having it be opt-out like the current PR implementation?
Also if we did keep the opt-out, I think I prefer an env var - which means we'd need to use an approach similar to Ruby where the release metadata is actually generated during bin/compile (where the ENV_DIR is available) and then loaded back off disk in bin/release.

I think keeping the opt-out might be better than opt-in, because the idea is, the buildpack would still work exactly the same as before(Unless opt-out of course).

I also prefer env vars, but I tested different approaches, none of them seem to have worked. I tried to use ENV_DIR, and it didn't seem to be available. I also tried using .profile.d, that also didn't seem to load the variable. 🤔 Any ideas?

[edmorley]

Thank you for the extra details :-)

And yes, it will be postgres, but it shouldn't matter exactly what DB we use outside of heroku

It does matter, since one of the solutions mentioned in #1056 (review) was to copy the Ruby buildpack and make the DB addition conditional on what packages were installed. ie: If the psycopg2 Python package is installed, then default to a Heroku Postgres DB. This would work fine for people say using MySQL with a third party Heroku addon or RDS MySQL for example, since they wouldn't have psycopg2 and so the DB wouldn't be added.

I also prefer env vars, but I tested different approaches, none of them seem to have worked. I tried to use ENV_DIR, and it didn't seem to be available. I also tried using .profile.d, that also didn't seem to load the variable. 🤔 Any ideas?

Yeah ENV_DIR is not available during bin/release, since the script is only passed the BUILD_DIR:
https://devcenter.heroku.com/articles/buildpack-api#bin-release

The workaround is to have bin/compile generate the release manifest (and use the environment variable to decide what to add in it), store it somewhere (eg in $BUILD_DIR/.heroku), then during bin/release the file is read back verbatim.

I've also started a discussion with the other language owners to see if they have any suggestions based on what the other languages do :-)

[BNMetrics]

It does matter, since one of the solutions mentioned in #1056 (review) was to copy the Ruby buildpack and make the DB addition conditional on what packages were installed. ie: If the psycopg2 Python package is installed, then default to a Heroku Postgres DB. This would work fine for people say using MySQL with a third party Heroku addon or RDS MySQL for example, since they wouldn't have psycopg2 and so the DB wouldn't be added.

But what if we do have psycopg2 installed, but we want to use an RDS postgres? In this case, the postgres addon will still be added automatically, and it still won't be ideal. It'd be nice if we can opt-out of the default addon by an env var.

Yeah ENV_DIR is not available during bin/release, since the script is only passed the BUILD_DIR:
https://devcenter.heroku.com/articles/buildpack-api#bin-release

I saw in bin/compile, the ENV_DIR var is exported. I guess bin/release runs on a different shell, so the ENV_DIR is not available. Is that correct?

The workaround is to have bin/compile generate the release manifest (and use the environment variable to decide what to add in it), store it somewhere (eg in $BUILD_DIR/.heroku), then during bin/release the file is read back verbatim.

Would we be able to copy the entirety of ENV_DIR to $BUILD_DIR/.heroku in bin/compile, if bin/release is able to read from it?

I've also started a discussion with the other language owners to see if they have any suggestions based on what the other languages do :-)

Thank you! 😸 I will let you know if I think of anything as well.

[edmorley]

But what if we do have psycopg2 installed, but we want to use an RDS postgres?

There are a few scenarios here:

1. app doesn't use Django
2. app uses Django + PostgreSQL, and wants to use Heroku Postgres (or is indifferent about what provider to use)
3. app uses Django + PostgreSQL, and wants to use a third party provider
4. app uses Django + MySQL/..., and so needs to use a different Heroku addon, or a third party provider outside of an addon
5. app uses Django but no DB

Scenario 1 and 2 are covered by the current implementation, since it checks for manage.py before using the Heroku Postgres addon.

We could add an "opt out" implementation (either using file or env var), which would additionally solve scenarios 3-5, however users would need to know about opt-out and actually do it, whereas it would be better for this to work out of the box if possible.

As such an alternative would be to have the default behaviour be to check for both manage.py and that a Postgres related package is installed - meaning all scenarios apart from (3) work out of the box.

That's not to say (3) is less important or necessarily something we should ignore, but this is the first time someone has asked about it (that I can find) since the auto DB feature was added in 2012 - so it was useful to know whether yours was this case or one of the others.

Out of curiosity, how are the credentials set for the DB on these apps? Via DATABASE_URL? At what point is it populated? If prior to the build, perhaps we could skip the postgres addon if DATABASE_URL is already set? This would make (3) work roughly out of the box too, without the need for a custom opt-out env var.

I saw in bin/compile, the ENV_DIR var is exported. I guess bin/release runs on a different shell, so the ENV_DIR is not available. Is that correct?

The detect, compile and release scripts are run in turn, and each is passed certain parameters. The release script is not passed the parameter that maps to env dir, so whilst it's theoretically on disk, the path isn't known. See:
https://devcenter.heroku.com/articles/buildpack-api#buildpack-api

Would we be able to copy the entirety of ENV_DIR to $BUILD_DIR/.heroku in bin/compile, if bin/release is able to read from it?

This wouldn't be recommended, since it would leak secrets into the slug (or cache). The method I mentioned is the way this would need to be implemented (and is the way the Ruby buildpack does it).

[BNMetrics]

Out of curiosity, how are the credentials set for the DB on these apps? Via DATABASE_URL? At what point is it populated? If prior to the build, perhaps we could skip the postgres addon if DATABASE_URL is already set? This would make (3) work roughly out of the box too, without the need for a custom opt-out env var.

DATABASE_URL(we actually name it differently in the django app itself) is env var populated on deploy time, before the build. The var is obtained from reference to the secret storage.

I did try populating DATABASE_URL as well, however, after building, the default db still gets created, but under a different env var, such as HEROKU_POSTGRESQL_SILVER_URL.

perhaps we could skip the postgres addon if DATABASE_URL is already set?

That would certainly be ideal, but how do we read, or know is DATABASE_URL is set?

[edmorley]

I did try populating DATABASE_URL as well, however, after building, the default db still gets created, but under a different env var, such as HEROKU_POSTGRESQL_SILVER_URL.

This is expected. I was meaning we'd still need to make changes to the buildpack, but that instead of using the custom PYTHON_BUILDPACK_NO_DEFAULT_ADDON (which users would need to discover/set themselves), we could use DATABASE_URL that may already be being set for such apps (or if not it's easy enough to document that).

how do we read, or know is DATABASE_URL is set?

By checking for the presence of ${ENV_DIR}/DATABASE_URL at some point during bin/compile. For an example of such usage, see:

heroku-buildpack-python/bin/steps/pip-install

Line 16 in dfbe8dd

if [[ -r "$ENV_DIR/PIP_EXTRA_INDEX_URL" ]]; then

Since this is restructuring the buildpack rather than just a one line change, I'd totally understand if you'd prefer me to make the change. Though this would need to go into my backlog and be prioritised accordingly, so I wouldn't be able to get to this immediately.

[oinopion]

I like the option of checking DATABASE_URL - you can even set it to empty string or something like null:// to solve problem for apps that don't need databases at all.

[edmorley]

@BNMetrics Hi!

I've filed two issues for tackling different parts of this topic:

1. Skip the default heroku-postgresql addon if a Postgres DB driver isn't installed #1067: "Skip the default heroku-postgresql addon if a Postgres DB driver isn't installed" (covers the "no DB" case in a less hacky way than setting DATABASE_URL to an empty string)
2. Skip the default heroku-postgresql addon if DATABASE_URL is set #1068: "Skip the default heroku-postgresql addon if DATABASE_URL is set" (I believe should cover the main issue this PR was aiming to address)

I've also synced both of them to our internal work tracker, so they appear in the overall backlog.

Do you have a sense of if you'd like to update this PR to address #1068 soon? If not, it might be best to close this PR out (since it's not ready for merge) and move the discussion to the issues pending either a new PR from the community, or the internal work items being prioritised in the backlog.

[edmorley]

Do you have a sense of if you'd like to update this PR to address #1068 soon? If not, it might be best to close this PR out (since it's not ready for merge) and move the discussion to the issues pending either a new PR from the community, or the internal work items being prioritised in the backlog.

Closing this out for now. Let's track potential solutions for the two different parts to this in the GitHub issues linked above.