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

When using aws-s3, force endpoints to be the same as Companion to simplify setup #5030

Closed
2 tasks done
Tracked by #4764
Murderlon opened this issue Mar 27, 2024 · 2 comments · Fixed by #5173
Closed
2 tasks done
Tracked by #4764

When using aws-s3, force endpoints to be the same as Companion to simplify setup #5030

Murderlon opened this issue Mar 27, 2024 · 2 comments · Fixed by #5173
Assignees
Labels
4.0 For the 4.0 major version Feature

Comments

@Murderlon
Copy link
Member

Initial checklist

  • I understand this is a feature request and questions should be posted in the Community Forum
  • I searched issues and couldn’t find anything (or linked relevant results below)

Problem

We strongly discourage using Companion just for S3 signing if aren't using Companion already. But when you need to implement it for you own backend, we require an embarrassing amount of almost 600 lines of complex code (client, server).

When you use companion, it's as simple as this:

uppy.use(AwsS3, {
  shouldUseMultipart: (file) => file.size > 100 * 2 ** 20,
  companionUrl: 'https://companion.uppy.io',
});

Solution

Why don't we force people to have the exact same endpoints as Companion? Then we only have to rename companionUrl and they can do this:

uppy.use(AwsS3, {
  shouldUseMultipart: (file) => file.size > 100 * 2 ** 20,
  endpoint: 'https://mycoolserver/uppy-s3',
});

Alternatives

Export the logic from the package so they only need to write a couple lines of code. This might make more sense if there are likely edge cases where people need to do other things than what our code does.

@Murderlon Murderlon added Feature 4.0 For the 4.0 major version labels Mar 27, 2024
@aduh95
Copy link
Contributor

aduh95 commented Mar 27, 2024

I think that's more or less what I suggested back in #4265.

@Murderlon
Copy link
Member Author

I still think companion-client should't be touched here and remain a strict API for Companion. But changing the option in aws-s3 is fine.

Can you think of any reason why people would need to deviate from our client-side code? Then the mentioned alternative may be better.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4.0 For the 4.0 major version Feature
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants