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

Fixed broken image serving in development server #202

Closed
wants to merge 1 commit into from

Conversation

crohkohl
Copy link

@crohkohl crohkohl commented Aug 7, 2015

Separation of changes in separate pull request: #199

On my machine the image serving, e.g. of the mean.jpg does not work.
The browser (tested IE and Chrome) cannot interpret the image probably due to the missing content type. The send_file function takes care of that all.

@lukeyeager
Copy link
Member

Thanks for the fix! Looks like I was working too hard.

Can we go ahead and use send_from_directory while we're at it? Then we wouldn't need any of the checks for path manipulation.
http://flask.pocoo.org/docs/0.10/api/#flask.send_from_directory

Also, watch your extra whitespace on line 246.

@crohkohl crohkohl mentioned this pull request Aug 7, 2015
@crohkohl
Copy link
Author

crohkohl commented Aug 7, 2015

send_from_directory sounds even better! Will you change that or should I do it?
I am still getting used to that white-space thin in python. All me editors use tabs ... I will need to adjust them.

@lukeyeager
Copy link
Member

I can't push commits to your fix_file_serving branch - you'll have to do that. FWIW, it seems to be this simple:

jobs_dir = config_value('jobs_dir')
return flask.send_from_directory(jobs_dir, path)

@crohkohl
Copy link
Author

crohkohl commented Aug 7, 2015

okay, I put in your suggested change and squashed the two commits

@lukeyeager
Copy link
Member

I fixed the docs merged this into master with 63a8456.

Thanks for the contribution!

@lukeyeager lukeyeager closed this Aug 7, 2015
@crohkohl crohkohl deleted the fix_file_serving branch August 7, 2015 23:09
@crohkohl
Copy link
Author

crohkohl commented Aug 7, 2015

great!

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