Fix regression caused by using newer version of Grape
Created by: dblessing
This commit fixes the regression caused by using a newer version of Grape and Grape entity. This solves Issue #4023 (closed).
I don't know if there was a particular reason for changing Grape version (commit 083d6656). If there is no compelling reason to change the Grape version, I recommend we revert that change with this PR to solve the current regression with raw content in the API. This regression breaks tools that integrate with the API.
Before:
After:
Merge request reports
Activity
Created by: NARKOZ
For broken raw content.
On Sunday, May 26, 2013, bke-drewb wrote:
Test for what?
— Reply to this email directly or view it on GitHubhttps://github.com/gitlabhq/gitlabhq/pull/4076#issuecomment-18456724 .
Nihad Abbasov
By Administrator on 2013-05-26T01:54:35 (imported from GitLab project)
By Administrator on 2013-05-26T01:54:35 (imported from GitLab)
Created by: bbodenmiller
@senny anyone who can give @bke-drewb some pointers on writing tests?
By Administrator on 2013-05-27T00:32:27 (imported from GitLab project)
By Administrator on 2013-05-27T00:32:27 (imported from GitLab)
Created by: dblessing
Not that I don't want to learn to write tests - I think this case should have a test. However, is it necessary to have a test prior to making a decision on this?
By Administrator on 2013-05-27T00:37:44 (imported from GitLab project)
By Administrator on 2013-05-27T00:37:44 (imported from GitLab)
Created by: bbodenmiller
I think we need to wait for @randx to let us know if the change in versions was for a particular reason. Tests would allow us to find this failure and prevent it from happening again as well as show that downgrading doesn't break anything.
By Administrator on 2013-05-27T00:39:42 (imported from GitLab project)
By Administrator on 2013-05-27T00:39:42 (imported from GitLab)
Created by: dblessing
Yeah, I'd like to hear if there was any reason for the upgrade. Also, I've been going back and forth on the Grape mailing list trying to see what we need to do to use the latest version and retain the functionality expected of raw/blob calls to the API.
By Administrator on 2013-05-27T00:41:46 (imported from GitLab project)
By Administrator on 2013-05-27T00:41:46 (imported from GitLab)
Created by: dzaporozhets
related to https://github.com/intridea/grape/issues/412
By Administrator on 2013-05-27T09:51:30 (imported from GitLab project)
By Administrator on 2013-05-27T09:51:30 (imported from GitLab)
Created by: dblessing
@randx Thanks for merging in the interim. I will definitely keep exploring options. I haven't been able to find any server side options yet. There are some that would work but would require the API user to set a 'format' parameter. I don't think that's the best option. I'll open an issue if I find out more.
By Administrator on 2013-05-27T12:41:46 (imported from GitLab project)
By Administrator on 2013-05-27T12:41:46 (imported from GitLab)
Created by: dzaporozhets
@bke-drewb fixed in grape master. See https://github.com/intridea/grape/issues/412 for discussion
By Administrator on 2013-05-27T17:28:05 (imported from GitLab project)
By Administrator on 2013-05-27T17:28:05 (imported from GitLab)
Created by: dblessing
@randx Great! I'll get a pull request going. One more question - for blob content it currently has content_type set to blob.mime_type. What happens is Ruby/HTML/etc files are then downloaded and/or rendered in the browser instead of viewed as plain text. What is the desired outcome for blob content? Simply changing this to text/plain will make it output the raw contents. Except, that likely won't work for images and such. Do you want to wait to integrate these changes until Grape releases a new version?
By Administrator on 2013-05-28T01:03:22 (imported from GitLab project)
By Administrator on 2013-05-28T01:03:22 (imported from GitLab)
