Skip to content

Resolve redirects early #117

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

Closed
icidasset opened this issue Jan 4, 2020 · 2 comments · Fixed by #118
Closed

Resolve redirects early #117

icidasset opened this issue Jan 4, 2020 · 2 comments · Fixed by #118
Assignees
Labels
enhancement New feature or request

Comments

@icidasset
Copy link
Contributor

For performance reasons, it would be ideal to resolve redirects of the given url.

Example scenario:

  • User provides url to Google Drive content
  • That url redirects to another url

Whenever this library makes a range request it uses the original url given by the user. But ideally we should used the "resolved redirect" url. There don't have to be much changes for this, we can get the resolved url from the initial HEAD/GET (fetch) request response.

// Basically
fetch().then(response => resolvedUrl = response.url)

(note: This may be resolved in the newer versions of this library, I should double check)

@Borewit
Copy link
Owner

Borewit commented Jan 5, 2020

This question leads to more questions.

Having a look to Diffuse dependencies, you are using remotestoragejs to sync or access Cloud audio files.

Why is this issue limited to Google Drive?

Just to get in sync with me. please have a look to @tokenizer/s3 and how this can be used.

Note that @tokenizer/s3 is utilizing the abstract module @tokenizer/range. So @tokenizer/range can be used to map to other chunk / range based clients.

Similar to the @tokenizer/s3, maybe it is worth to implement a @tokenizer/google-drive?

Or use an abstract storeage provider in betweeen, like:

Having a quick look to those API's, I don't see a way to read a range. This is the way, to make music-metadata(-browser) read the metadata efficient over a wide area network.

To understand where we are now, in you current implementation, can you confirm you actually utilizing the HTTP-range mechanism @icidasset?

@Borewit Borewit self-assigned this Jan 5, 2020
@Borewit Borewit added the enhancement New feature or request label Jan 5, 2020
@icidasset
Copy link
Contributor Author

Sorry if I caused any confusion, this is not limited to Google Drive at all (hence "example scenario").


Diffuse definitely uses the HTTP-range mechanism, and only needs tokenizer-http. tokenizer-s3 is cool, but not needed in Diffuse. Because Diffuse needs to do other things with S3 as well, like making a file tree. Does that make sense?

So for me personally, no need to implement @tokenizer/google-drive or other variants. Could be useful to other people perhaps, but not for Diffuse 😉

Sidenote: Remotestorage is optionally used in Diffuse to store the user's data, such as playlists, it's not used for music storage. With optionally I mean, the user chooses specifically where they want to store their data, remoteStorage is one option (see Diffuse's sign-in screen).


To come back to the original issue.
Let me make a quick PR for this, I think this'll be super easy to solve.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants