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

Refactor S3 asset URL handling #1085

Merged
merged 1 commit into from
Oct 1, 2020
Merged

Refactor S3 asset URL handling #1085

merged 1 commit into from
Oct 1, 2020

Conversation

edmorley
Copy link
Member

@edmorley edmorley commented Sep 30, 2020

Previously the buildpack's S3 bucket was defined in two places - once in VENDOR_URL and again during the pip installation step. This duplication was necessary since VENDOR_URL also contained the stack's name, whereas the pip use-case used a non-stack-specific S3 key prefix.

In order to:

  • reduce this duplication
  • simplify this buildpack's S3 bucket migration (where we'll soon be needing the vary the bucket name and wouldn't want to have to duplicate that logic in multiple places)
  • allow overriding of the URL for the pip use-case

...the VENDOR_URL variable has been replaced with S3_BASE_URL which no longer contains the stack name.

The user-configurable override has similarly been renamed from BUILDPACK_VENDOR_URL to BUILDPACK_S3_BASE_URL. Note: As before, this override cannot be set via standard app variables (see #989).

The unused USE_STAGING_BINARIES environment variable has been removed, since it's a leftover from the project to stand up a staging S3 bucket. It's redundant given the BUILDPACK_S3_BASE_URL variable.

Closes W-8142401.

@edmorley edmorley self-assigned this Sep 30, 2020
@edmorley edmorley marked this pull request as ready for review September 30, 2020 20:21
@edmorley edmorley requested a review from a team as a code owner September 30, 2020 20:21
Previously the buildpack's S3 bucket was defined in two places - once
in `VENDOR_URL` and again during the pip installation step. This
duplication was necessary since `VENDOR_URL` also contained the stack's
name, whereas the pip use-case used a non-stack-specific S3 key prefix.

In order to:
* reduce this duplication
* allow overriding of the URL for the pip use-case
* simplify this buildpack's S3 bucket migration (where we'll soon be
  needing the vary the bucket name and wouldn't want to have to
  duplicate that logic in multiple places)

...the `VENDOR_URL` variable has been replaced with `S3_BASE_URL` which
no longer contains the stack name.

The user-configurable override has similarly been renamed from
`BUILDPACK_VENDOR_URL` to `BUILDPACK_S3_BASE_URL`. Note: As before,
this override cannot be set via standard app variables (see #989).

Closes @W-8142401@.
@edmorley edmorley force-pushed the refactor-s3-url-handling branch from c02d3c8 to a24e289 Compare September 30, 2020 20:46
@edmorley edmorley merged commit b74a413 into main Oct 1, 2020
@edmorley edmorley deleted the refactor-s3-url-handling branch October 1, 2020 09:13
dryan pushed a commit to dryan/heroku-buildpack-python that referenced this pull request Nov 19, 2020
Previously the buildpack's S3 bucket was defined in two places - once
in `VENDOR_URL` and again during the pip installation step. This
duplication was necessary since `VENDOR_URL` also contained the stack's
name, whereas the pip use-case used a non-stack-specific S3 key prefix.

In order to:
* reduce this duplication
* simplify this buildpack's S3 bucket migration (where we'll soon be
  needing the vary the bucket name and wouldn't want to have to
  duplicate that logic in multiple places)
* allow overriding of the URL for the pip use-case

...the `VENDOR_URL` variable has been replaced with `S3_BASE_URL` which
no longer contains the stack name.

The user-configurable override has similarly been renamed from
`BUILDPACK_VENDOR_URL` to `BUILDPACK_S3_BASE_URL`. Note: As before,
this override cannot be set via standard app variables (see heroku#989).

The unused `USE_STAGING_BINARIES` environment variable has been
removed, since it's a leftover from the project to stand up a staging S3 bucket.
It's redundant given the `BUILDPACK_S3_BASE_URL` variable.

Closes @W-8142401@.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants