Provision a Heroku Postgres DB in fewer cases #1363
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
As part of the Buildpack API, a buildpack can request the provisioning of addons on the first build of an app via the `bin/release` payload. (This feature does not apply to subsequent builds of existing apps.) Previously the Python buildpack requested a Heroku Postgres DB be provisioned if the app source contained a `manage.py` file in the source root or any subdirectory up to three levels deep. This meant a Postgres DB was provisioned even in cases where the app wasn't a Django app (but just happened to have a `manage.py` script) or used an alternative DB backend (eg MySQL), or didn't use a DB at all. Now, the DB is only provisioned if all of the following are true: - a `manage.py` file exists in the root of the app source - `django` is installed - `psycopg2` is installed This ensures it's still provisioned in the Python getting-started guide case (and any other similar cases), but not in the other scenarios mentioned above. Fixes #1067. GUS-W-8098791.
Malax
approved these changes
Sep 22, 2022
This was referenced Oct 3, 2022
edmorley
added a commit
that referenced
this pull request
Oct 4, 2022
In #1363 the logic for determining when to automatically provision a Heroku Postgres database addon was adjusted to reduce false positives. Unfortunately, this broke provisioning in all cases, since it turns out that the build system does not source the `export` script prior to running `bin/release` (like it does when running subsequent buildpacks), and so the buildpack configured env vars (such as `PATH` are not set. This meant that when `bin/release` used the `is_module_available` utility, it was calling system Python (where the Django and psycopg2 packages were not installed) and so always failing the conditional. Unfortunately we do not currently have an easy way to test `bin/release`, otherwise a test case would have been added in the original PR. I had manually tested the original PR using the Python getting started guide, but had only checked via accessing the app and trying to run the Django `./manage.py migrate` command. However it seems the Django config for that project silently falls back to sqlite if `DATABASE_URL` is not set - so the project still worked. For this PR I have manually tested to ensure the addon appears on the app. GUS-W-11851647.
edmorley
added a commit
that referenced
this pull request
Oct 4, 2022
In #1363 the logic for determining when to automatically provision a Heroku Postgres database addon was adjusted to reduce false positives. Unfortunately, this broke provisioning in all cases, since it turns out that the build system does not source the `export` script prior to running `bin/release` (like it does when running subsequent buildpacks), and so the buildpack configured env vars (such as `PATH` are not set. This meant that when `bin/release` used the `is_module_available` utility, it was calling system Python (where the Django and psycopg2 packages were not installed) and so always failing the conditional. Unfortunately we do not currently have an easy way to test `bin/release`, otherwise a test case would have been added in the original PR. I had manually tested the original PR using the Python getting started guide, but had only checked via accessing the app and trying to run the Django `./manage.py migrate` command. However it seems the Django config for that project silently falls back to sqlite if `DATABASE_URL` is not set - so the project still worked. For this PR I have manually tested to ensure the addon appears on the app. Fix #1374. GUS-W-11851647.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
As part of the Buildpack API, a buildpack can request the provisioning of addons on the first build of an app via the
bin/releasepayload. (This feature does not apply to subsequent builds of existing apps.)Previously the Python buildpack requested a Heroku Postgres DB be provisioned if the app source contained a
manage.pyfile in the source root or any subdirectory up to three levels deep.This meant a Postgres DB was provisioned even in cases where the app wasn't a Django app (but just happened to have a
manage.pyscript) or used an alternative DB backend (eg MySQL), or didn't use a DB at all.Now, the DB is only provisioned if all of the following are true:
manage.pyfile exists in the root of the app sourcedjangois installedpsycopg2is installedThis ensures it's still provisioned in the Python getting-started guide case (and any other similar cases), but not in the other scenarios mentioned above.
Fixes #1067.
GUS-W-8098791.