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

Modify endpoints in own server section of AWS S3 #214

Merged
merged 2 commits into from Apr 2, 2024

Conversation

nerg4l
Copy link
Contributor

@nerg4l nerg4l commented Mar 27, 2024

Copy link
Member

@Murderlon Murderlon left a comment

Choose a reason for hiding this comment

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

Thanks for making a PR.

Now that I think about it, does it make sense to duplicate endpoints of the example? I feel like it's just likely to get out of date again.

Some alternatives:

  1. Instead of writing the endpoints and a link the example root, we can also create to steps:
    1. Setup Uppy client
    2. Setup your server
  2. Or we keep the HTTP methods but remove the specific endpoints and add a sentence above that you need you need these endpoints but it's up to you to decide under which route they fall, as long as you keep it in sync with Uppy.

What do you think?

@nerg4l
Copy link
Contributor Author

nerg4l commented Mar 28, 2024

I'm still getting familiar with the available Uppy components.

These endpoints will only change if @uppy/companion changes, am I wrong?

I think @uppy/aws-s3-multipart could be used for the "Setup Uppy client". That component has a predefined set of routes.

Also, @uppy/companion has great documentation for the routes. Example: https://github.com/transloadit/uppy/blob/960362b373666b18a6970f3778ee1440176975af/packages/%40uppy/companion/src/server/controllers/s3.js#L36-L50
Maybe it's enough to put these somewhere on uppy.io.

@Murderlon
Copy link
Member

These endpoints will only change if @uppy/companion changes, am I wrong?

No we are updating the section on using it with your own server. At the moment the endpoints don't matter at all. For instance in createMultipartUpload you have to use fetch to your own backend. You're in control what endpoint you fetch, not Uppy.

As discussed before we should change the property name from companionUrl to endpoint and document what to do exactly so you don't have to write this all manually. But until this in there, we need to write docs for the current reality, brining me back to my two suggestions above.

@nerg4l
Copy link
Contributor Author

nerg4l commented Mar 28, 2024

From a user's point of view, I think the first option would be better.

@Murderlon Murderlon linked an issue Mar 29, 2024 that may be closed by this pull request
@Murderlon
Copy link
Member

Would you like to make the change?

@nerg4l
Copy link
Contributor Author

nerg4l commented Mar 29, 2024

I will update this PR later today. I can only work on it outside of European working hours.

@Murderlon Murderlon enabled auto-merge April 2, 2024 09:31
@Murderlon Murderlon added this pull request to the merge queue Apr 2, 2024
Merged via the queue into transloadit:main with commit 610bbe8 Apr 2, 2024
2 checks passed
@nerg4l nerg4l deleted the aws-s3-own-server branch April 2, 2024 09:45
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.

Use with your own server section of AWS S3 contains misleading routes
2 participants