search

astaxie / beegae

beego for GAE
Other
38 stars 11 forks source link

Path to the blob uploading fix. #16

Open ghost opened 9 years ago

ghost commented 9 years ago

As I mention in issue #15 with blobstore service.

Notice that I didn't test this fix on Google Cloud Storage case. Maybe someone who uses it could test it by themself.

ghost commented 9 years ago

6 days now. Is anyone want to discuss about this issue or may be I just live with it?

someone1 commented 9 years ago

Sorry, I'm in the middle of a move and haven't had time to setup my dev environment. I'll have to update the beegae base for the recent beego update and will look into this merge request right after.

ghost commented 9 years ago

@someone1

OK then. I'm not in rush and I can wait.

Notice that I submit this commit for discuss this problem. The bug has been nailed by the way.

The code style I used in that commit is different. My editor automatically did that style convert (Tab -> Space) and I never realized that until I did the commit (Sorry for that). You may need to change it before merge.

But I bet you will have better solution coming out.

Anyway, the information I discovered all in this commit and #15, you may look into those. If you want anything cleared or changed, please send me a comment.

yosukesuzuki commented 9 years ago

I tried this patch and works fine for blobstore handler and normal post request.

fork version from latest beegae https://github.com/yosukesuzuki/beegae

my application https://github.com/yosukesuzuki/goisky-tools

someone1 commented 9 years ago

I've been giving this a lot of thought and I'm not happy with any of my conclusions.

The solution proposed here would work, though the logic plays out to if CopyRequestBody || context.Input.IsUpload() which for non-blobstore uploads you would not need to (or want to) copy the request body.

There is no sure way of knowing if we are handling a blobstore redirect in the router so we can't put anything there. We could update the router to always try and parse as if the request were for the blobstore, but as aforementioned we don't know when we are dealing with a blobstore upload or not and that would be extra work per request.

The information we require IS available withcontroller.GetFile(key string) however, the blobstore metadata would be encoded as mime headers in the response and would need to be parsed out. This could simply just mean copy the blobstore.ParseUpload parts to a function that accepts just these results for parsing, but that's not an ideal integration withe AppEngine.

I want to give this some more thought and am open to further discussion on the topic.