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

adding file_name option to make_response #8

Merged
merged 2 commits into from
Jan 13, 2016

Conversation

abehrens
Copy link

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):
Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Author

Choose a reason for hiding this comment

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

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.

Copy link
Member

Choose a reason for hiding this comment

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

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

@chfw
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
credit goes to @abehrens, adding file_name option to make_response
@chfw chfw merged commit 2d5fd9a into pyexcel-webwares:master Jan 13, 2016
@abehrens
Copy link
Author

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
Copy link
Member

chfw commented Jan 17, 2016

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

@abehrens
Copy link
Author

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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants