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

'Upload from URL' routes #28

Merged
merged 6 commits into from
Apr 12, 2020
Merged

Conversation

akhil-rana
Copy link
Contributor

@akhil-rana akhil-rana commented Apr 10, 2020

Two new routes have been added.
One for uploading image file from the given URL.
The other for checking that whether the image upload has been completed or not (is useful for large image files or less bandwidth connection)

This is used with caMicroscope #353

@akhil-rana akhil-rana changed the title Upload from URL routes 'Upload from URL' routes Apr 10, 2020
@birm
Copy link
Member

birm commented Apr 10, 2020

Request a review or otherwise let me know when you're ready with these two PRs :)

@akhil-rana
Copy link
Contributor Author

@birm I think they are ready.
Please review them both.

@birm birm self-requested a review April 10, 2020 18:05
SlideServer.py Outdated Show resolved Hide resolved
@birm
Copy link
Member

birm commented Apr 10, 2020

Can we give the user the ability to set an auth header for the request? e.g. for google bucket

@akhil-rana
Copy link
Contributor Author

akhil-rana commented Apr 11, 2020

I'm not sure that I understand. Can you elaborate a little.

@birm
Copy link
Member

birm commented Apr 11, 2020

In short, add a url param (or something) that lets a user specify an Authorization header to the url.

This is for when a url requires a specific auth header to access. In the simple case, let's pretend we're grabbing a slide from another camicroscope instance, then it needs an auth token for that instance. (though while camic can accept it as a urlparam, some sources may not).

@birm
Copy link
Member

birm commented Apr 11, 2020

That said, if you don't want to do this now, that's fine; this is an improvement even without that functionality.

@akhil-rana
Copy link
Contributor Author

Maybe workflow for the authorization header will be different for every service like google and any other. If it's alright then lets keep this option to deal with the links which directly(publicly) points to the file for now.

@akhil-rana
Copy link
Contributor Author

However, just a thought;
maybe we can work on something like "upload from google drive" in future.
But I think it will require an API key from google. Maybe we can let the user set his/her own credentials.json file.

@akhil-rana akhil-rana requested a review from birm April 11, 2020 16:51
@birm
Copy link
Member

birm commented Apr 12, 2020

I'm getting a 404 on the new route (http://localhost:4010/loader/urlupload/continue/BIYBJ2HZH3). Maybe I've messed up testing. I'll look again soon with more energy.

@akhil-rana
Copy link
Contributor Author

akhil-rana commented Apr 12, 2020

url has to be passed as body in the POST request like:

{ 'url' : 'http....(encoded)' }

@akhil-rana
Copy link
Contributor Author

Anyway even if url is not passed, 404 should not occur.🤔
Works for me perfectly. If you encounter something again. Do let me know.

Copy link
Member

@birm birm left a comment

Choose a reason for hiding this comment

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

Silly me, I'd built the loader against the wrong pull request... Seems to work fine! Thanks for working through this!

@birm birm merged commit 4623d7d into camicroscope:develop Apr 12, 2020
@akhil-rana akhil-rana deleted the uploadFromURL branch June 18, 2020 21:13
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