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

Better URLs #15

Open
rhsimplex opened this issue Jun 1, 2017 · 3 comments
Open

Better URLs #15

rhsimplex opened this issue Jun 1, 2017 · 3 comments

Comments

@rhsimplex
Copy link
Contributor

Visualization is determined with a cookie now, but a URL would be the obviously better way.

Also per @knub

Feature idea:
Would also be nice to have somewhat RESTful urls, e.g.
http://127.0.0.1:5000/occlusion?strides=20&window=0.5. This way, one could quickly check different params by just changing the URL instead of navigating back and forth. The image itself would still need to be stored in the session (or just take "the last" image) in this case.

@jan-xyz
Copy link
Contributor

jan-xyz commented Jun 1, 2017

This is actually the issue I talked to @rrothe about. I spend the entire day thinking about a good path to choose, especially with only some visualisers having settings. I have a branch currently that breaks it down. It kind of goes like this:
vis w/ settings:/ -> /visualisation_settings -> /select_files -> /result
vis w/o settings: / -> /select_files -> /result

It is already an improvement because it adds /result but it still feels clunky. In my opinion it would make more sense if you select the file first (that's what you want to check anyways, right?), and maybe even have a history of files to pic from which would bring you to this:
/ -> /file_picker -> /visualiser -> /result
/visualiser is a dynamic form that changes based on the item you pick with optional settings. It is more complex to display visualiser settings though. It would require a dynamic form being POSTed and for that you'd need Javascript. If you use Javascript here, it would even be possible to cut it down further to just:
/ -> /result
In that case / would just be a dynamic form with a file picker. I am currently looking into how to do this with Javascript. It'd break a few of the unit tests but probably also simplify picasso.py a bit.

I like the idea of being RESTful with this. That would allow scripted access to the service as well! IMHO, the REST interface should be implemented next to the frontend you currently have, though. I see the advantage of both.

I hope this isn't too much of a brain dump. Let me know if my thoughts help :)

If you want to see how my branch behaves you can check it out here: https://github.com/jan-xyz/picasso/tree/make_files_reusable

@rhsimplex
Copy link
Contributor Author

Good suggestions for sure --I'm not really a web developer, and it definitely shows in the structure of this application! RESTful URLs would really professionalize the project a bit, and make it a bit easier to write tests -- yes we'll need to update them, but I can work with you on that.

I like the idea of selecting the file(s) first, but I wasn't able to get that working probably for the reason you described.

If you're tackling this, there's a couple knock-on issues that are a little embarrassing that you may consider looking at. In the file upload and download I ran into the problem of browsers caching files, since they would have the exact same path. I worked around this by adding a timestamp to the filename, but I think that's really inelegant. Maybe you can suggest a better way?

@jan-xyz
Copy link
Contributor

jan-xyz commented Jun 2, 2017

with RESTful URLs you mean having query strings? Or do you mean the whole thing being actually RESTful? In order for it to be RESTful you'd need to hit these requirements.

I wrote up a small POC that works with query strings (example given by @knub; I added image UID):

from flask import Flask, jsonify, request

# Initialize the Flask application
app = Flask(__name__)


@app.route('/result')
def result():
    visualizer = request.args.get('visualizer')
    image = request.args.get('image')
    if visualizer == 'occlusion':
        stride = request.args.get('stride')
        if stride is None:
            stride = 20
        window = request.args.get('window')
        if window is None:
            window = 0.5
        return jsonify(vis=visualizer, image=image, stride=stride, window=window)
    return jsonify(vis=visualizer, image=image)

if __name__ == '__main__':
    app.run(
        host="0.0.0.0",
        port=int("8080")
    )

If you GET http://localhost:8080/result?visualizer=occlusion&image=UID_of_image_here&stride=30&window=1 you can play with the parameters. The POC only supports the occlusion visualiser to have settings. For it to work this way you'd need a separate step before to PUT images and respond with the UID for that image.

Regarding the images, I would suggest storing them under their hashed value (maybe md5). That way you can be sure that even if you get an image called IMG_0001.jpg twice with the same timestamp, they won't be overwriting each other.

I am no expert in web development, I usually work in networking and IT automation, but I am certainly interested to work on this and get my hands dirty 👍

@jan-xyz jan-xyz mentioned this issue Jun 2, 2017
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

No branches or pull requests

2 participants