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

Feature :- Upload files by URL completed #1658

Closed

Conversation

MohdImran001
Copy link
Contributor

Fixes #387

Hi @catarak, I have completed the Upload files by URL feature mentioned in issue #387. Please review this PR and let me know if it needs some changes.

@MohdImran001
Copy link
Contributor Author

Hi @catarak, you have not given me any feedback for this PR. Please review it.

@catarak
Copy link
Member

catarak commented Nov 20, 2020

Hey @MohdImran001, sorry for the delay on reviewing this PR! I think the way that you've approached this issue is great and works. I had a different idea about how to approach this issue, and there's a shortcoming from the way you've done it here, which I will explain:

In p5.js, when you use the function loadImage(), it makes an AJAX request to fetch that image. This works fine if your sketch and the image are hosted on the same domain; however, if they are in different domains, there could be CORS issues, depending on how the server that is hosting the image is configured.

The way that you have implemented the upload by URL feature, the image is still using the same URL, versus uploading it to the S3 bucket hosted by the p5.js Editor. Sometimes this is okay and works, but sometimes the URL is not CORS enabled and doesn't work. I think there's a few different ways to fix this:

  1. Use the URL to download the image, and then upload it to the p5.js Editor S3 bucket. Save that URL to the sketch.
  2. Check if the URL is CORS enabled for the p5.js Editor. If it is, use the original URL. If it is not, upload the image to the S3 bucket. One issue that could come up with this approach is if the CORS configuration for a URL changes to be more restrictive. Hosting files in the S3 bucket is not free, so that is one plus on this approach.
  3. Use CORS anywhere. Don't upload any of the images to the S3 bucket. In my experience, proxying through this CORS anywhere server is slow, but maybe it would make sense to host an instance just for the web editor.

I don't expect you to figure out the right approach to this—I just wanted to communicate what I'm thinking through and why I haven't tackled this yet! Thanks for working on this issue.

@MohdImran001
Copy link
Contributor Author

Hi @catarak, Thank you for your comments. I really appreciate it. I am not an expert but I feel that using CORS anywhere is the most efficient way to solve this problem. I think that the cost of hosting an instance for the web editor is low as compared to hosting many files in the S3 bucket. Whatever approach you decide to tackle this issue, I request you to let me know about it and I will work on it because I am learning a lot by contributing to this project. Again, thank you for your reply.

@catarak
Copy link
Member

catarak commented Nov 23, 2020

@MohdImran001 thanks for your input! I think one thing to explore going forward would be to test using a self-hosted cors anywhere instance and figure out how fast/slow it is.

@MohdImran001
Copy link
Contributor Author

Sure @catarak, I will explore it by hosting it on Heroku. Do you have any suggestions on which tool or library should I use to check its speed?

@catarak catarak closed this Dec 9, 2021
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.

Upload files by URL
2 participants