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

Compatibility with grape 2.2.0 #939

Closed
padde opened this issue Sep 16, 2024 · 6 comments · Fixed by #940
Closed

Compatibility with grape 2.2.0 #939

padde opened this issue Sep 16, 2024 · 6 comments · Fixed by #940

Comments

@padde
Copy link
Contributor

padde commented Sep 16, 2024

Running on the latest 2.2.0 release of grape will produce these exceptions:

NameError:
  uninitialized constant Grape::ContentTypes::CONTENT_TYPES
# ./lib/grape-swagger/endpoint.rb:15:in `content_types_for'

NoMethodError:
  undefined method `formatters' for module Grape::Formatter
# ./lib/grape-swagger/endpoint.rb:14:in `content_types_for'

Used here:

Grape has removed/renamed these constants/methods via ruby-grape/grape@fb67ea9#diff-e28e889e3579545f8e2fedc3ab52f78530206ed808afdc9a23703f8b089f80e3

@padde
Copy link
Contributor Author

padde commented Sep 16, 2024

I'd love to work on a fix, however I am not sure what the correct solution would be here. I'd be grateful for any guidance.

Here is a first attempt: #939

padde added a commit to padde/grape-swagger that referenced this issue Sep 16, 2024
Running on grape 2.0.2 resulted in the following exceptions:

    NameError:
      uninitialized constant Grape::ContentTypes::CONTENT_TYPES
    # ./lib/grape-swagger/endpoint.rb:15:in `content_types_for'

    NoMethodError:
      undefined method `formatters' for module Grape::Formatter
    # ./lib/grape-swagger/endpoint.rb:14:in `content_types_for'

This is due to internal refactorings in grape as of v2.0.2, specifically
ruby-grape/grape@fb67ea99

Fixes ruby-grape#939
padde added a commit to padde/grape-swagger that referenced this issue Sep 16, 2024
Running on grape 2.0.2 resulted in the following exceptions:

    NameError:
      uninitialized constant Grape::ContentTypes::CONTENT_TYPES
    # ./lib/grape-swagger/endpoint.rb:15:in `content_types_for'

    NoMethodError:
      undefined method `formatters' for module Grape::Formatter
    # ./lib/grape-swagger/endpoint.rb:14:in `content_types_for'

This is due to internal refactorings in grape as of v2.0.2, specifically
ruby-grape/grape@fb67ea99

Fixes ruby-grape#939
padde added a commit to padde/grape-swagger that referenced this issue Sep 16, 2024
Running on grape 2.2.0 resulted in the following exceptions:

    NameError:
      uninitialized constant Grape::ContentTypes::CONTENT_TYPES
    # ./lib/grape-swagger/endpoint.rb:15:in `content_types_for'

    NoMethodError:
      undefined method `formatters' for module Grape::Formatter
    # ./lib/grape-swagger/endpoint.rb:14:in `content_types_for'

This is due to internal refactorings in grape as of v2.2.0, specifically
ruby-grape/grape@fb67ea99

Fixes ruby-grape#939
@olivier-thatch
Copy link

We just ran into this after Dependabot upgraded the grape gem.

Unless a fix is going to be released soon, I would recommend releasing a new patch version that updates the gemspec to change the < 3.0 grape constraint to < 2.2.

@padde
Copy link
Contributor Author

padde commented Sep 17, 2024

So I had started working on a fix for this but apparently it's not as easy to solve as I thought. What I've found so far:

grape-swagger references some internal constants and methods of grape regarding content types and formatters, which were removed/renamed with its 2.2.0 release.

@dblock suggested just copying over those constants from grape which I have done in my PR but this should really only be seen as a temporary fix.

I am saying this because from what I understand, referencing these constants was never really giving us the expected result in the first place. Grape allows specifying custom content types and formatters in the DSL, which seem to have been disregarded by grape-swagger the whole time. Personally, I would not attempt to fix this in the same PR as we might have to spend a considerable amount of work to come up with a solution that actually works for older and newer grape versions as well as new test cases. I think we should treat this issue as a separate bug.

@padde
Copy link
Contributor Author

padde commented Sep 17, 2024

I see this issue has already been reported a few years ago, for reference: #700

@dblock
Copy link
Member

dblock commented Sep 18, 2024

@padde You're absolutely right. Fixing #700 is beyond the basic compat scope, so let's keep it that way. Thanks for picking the simple fix up.

@dblock
Copy link
Member

dblock commented Sep 21, 2024

I released 2.1.1.

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 a pull request may close this issue.

3 participants