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

S3ServerSide for both local/live envs, env vars for local endpoints #166

Merged
merged 4 commits into from
Jun 9, 2020

Conversation

LoneRifle
Copy link
Contributor

Problem

S3LocalDev and S3ServerSide differ significantly only by the S3
API client, and the prefix for the short URL. Apply DRY to the two
classes so that the same code is exercised both in local and live
environments, injecting appropriate state into S3ServerSide at
runtime

Blocks #123

Solution

  • DependencyIds: Define new constants for s3Client and fileURLPrefix

  • S3ServerSide: Use injectable ctor parameters instead of module-level
    constants

  • inversify.config: Rework config so that only S3ServerSide inputs
    are determined by NODE_ENV, not the entire s3 dependency

  • api/user: Use the s3 held by the container, not the methods
    it destructures into, so that we can properly refer to the object's
    state through this

  • Rework aws.test.ts to account for changes

  • Remove S3LocalDev, no longer used

  • feat: allow S3 endpoints to be controlled by env vars

Tests

aws.test.ts now fully fleshed out with mocks

Deploy Notes

Deploy to staging to verify that Go still works as intended

New environment variables:

  • BUCKET_ENDPOINT : the endpoint that Go will use to talk to S3 when uploading files/acls
  • ACCESS_ENDPOINT : used to generate the long URLs for file links

LoneRifle added 3 commits June 9, 2020 09:08
S3LocalDev and S3ServerSide differ significantly only by the S3
API client, and the prefix for the short URL. Apply DRY to the two
classes so that the same code is exercised both in local and live
environments, injecting appropriate state into S3ServerSide at
runtime

- DependencyIds: Define new constants for s3Client and fileURLPrefix
- S3ServerSide: Use injectable ctor parameters instead of module-level
  constants
- inversify.config: Rework config so that only S3ServerSide inputs
  are determined by `NODE_ENV`, not the entire s3 dependency
- `api/user`: Use the s3 held by the container, not the methods
  it destructures into, so that we can properly refer to the object's
  state through `this`
- Rework `aws.test.ts` to account for changes
- Remove S3LocalDev, no longer used
- Allow the developer to override the endpoint for local dev environment
- Default these to localstack/localhost on port 4566, the edge port
@LoneRifle LoneRifle requested review from yong-jie and kylerwsm June 9, 2020 02:33
Copy link
Contributor

@liangyuanruo liangyuanruo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks great! thanks

@LoneRifle LoneRifle merged commit 0c156d6 into develop Jun 9, 2020
@LoneRifle LoneRifle deleted the global-s3 branch June 9, 2020 03:06
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