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

adding file_name option to make_response #8

Merged
merged 2 commits into from Jan 13, 2016

Conversation

Projects
None yet
2 participants
@abehrens
Copy link

abehrens commented Jan 13, 2016

I wanted a convenient way to provide the file name to be presented in the content-disposition header instead of passing it in via a form.

This overrides the proxied pyexcel make* methods and adds an extra (default none) file_name option. This could also be accomplished with a decorator, either in the client's code or within init.py, but I decided this was clearer.

Only the name of the file, not the extension, needs to be provided. The extension will be added based on the file_type argument passed in.

)

__VERSION__ = '0.0.3'
def add_file_name(response, file_name, file_type):

This comment has been minimized.

@chfw

chfw Jan 13, 2016

Member

These code changes look a bit lengthy. There must be a simpler way to do this. Please allow me to re-factor it along aside with pyexcel-webio.

This comment has been minimized.

@abehrens

abehrens Jan 13, 2016

Author

I agree. A decorator could be provided if the file name wont change based on the request. For example:

@download_file_name("exported_data")
@app.route("/download")
def download():
  return excel.make_response(....

But itd be beneficial to allow the file name arg to vary based on the request. A decorator could be applied to the proxied pyexcel methods, too.

This comment has been minimized.

@chfw

chfw Jan 13, 2016

Member

I have done the code refactoring. Feel free to comment or contribute

@@ -100,11 +100,15 @@ Here are some example codes::
<p><input type=file name=file><input type=submit value=Upload>
</form>
'''

@app.route("/download", methods=['GET'])

This comment has been minimized.

@chfw

chfw Jan 13, 2016

Member

I will remove this old 'download' code so that it looks light weight.

@@ -44,3 +44,10 @@ def test_download(self):
array = sheet.to_array()
assert array == self.data

def test_override_file_name(self):

This comment has been minimized.

@chfw

chfw Jan 13, 2016

Member

I like this

@chfw

This comment has been minimized.

Copy link
Member

chfw commented Jan 13, 2016

Overall, I welcome your changes. Meanwhile, I will refactor the code changes with pyexcel-webio.

chfw added a commit that referenced this pull request Jan 13, 2016

Merge pull request #8 from abehrens/file_name_override
credit goes to @abehrens, adding file_name option to make_response

@chfw chfw merged commit 2d5fd9a into pyexcel-webwares:master Jan 13, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@abehrens

This comment has been minimized.

Copy link
Author

abehrens commented Jan 13, 2016

Thanks. I know there are a few ways to go about this but the fancier I tried to get with it via a decorator the less I liked it. Not wedded to any one approach, as long as it's simple for the client to specify a file name, I'll be happy.

@chfw chfw referenced this pull request Jan 13, 2016

Closed

set download file name #2

chfw added a commit that referenced this pull request Jan 13, 2016

@chfw

This comment has been minimized.

Copy link
Member

chfw commented Jan 17, 2016

This patch is released along with pyexcel-webio and others.

@abehrens

This comment has been minimized.

Copy link
Author

abehrens commented Jan 17, 2016

Great! Thank you.

On Saturday, 16 January 2016, jaska notifications@github.com wrote:

This patch is released along with pyexcel-webio and others.


Reply to this email directly or view it on GitHub
#8 (comment).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment