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

Reconsider SafeYAML gem dependency #249

Open
olleolleolle opened this issue Feb 24, 2020 · 4 comments
Open

Reconsider SafeYAML gem dependency #249

olleolleolle opened this issue Feb 24, 2020 · 4 comments

Comments

@olleolleolle
Copy link
Member

This Issue is extracted from #241.


I think, SafeYAML is also deprecated in Ruby 2.3+.
Use YAML.safe_load instead of SafeYAML.load.

class ParseYaml < ResponseMiddleware
  define_parser do |body, parser_options|
    YAML.safe_load(body, **(parser_options || {}))
  end
end

But there are two problems.

SafeYAML and YAML's option is not compatible.

This makes a breaking change.

  • SafeYAML
    • :deserialize_symbols
    • :whitelisted_tags
    • :custom_initializers
    • :raise_on_unknown_tag
  • YAML
    • :permitted_classes
    • :permitted_symbols
    • :aliases
    • :filename
    • :fallback
    • :symbolize_names

YAML.safe_load option is not compatible in Ruby 2.3-2.7

Supporting all of this is a bit too complicated.

2.3, 2.4:

def self.safe_load yaml, whitelist_classes = [], whitelist_symbols = [], aliases = false, filename = nil

2.5: symbolize_names is added as kwargs

def self.safe_load yaml, whitelist_classes = [], whitelist_symbols = [], aliases = false, filename = nil, symbolize_names: false

2.6+: all arguments are now kwargs

def self.safe_load yaml, permitted_classes: [], permitted_symbols: [], aliases: false, filename: nil, fallback: nil, symbolize_names: false
@d-m-u
Copy link
Contributor

d-m-u commented Apr 4, 2020

hey @olleolleolle I'd be happy to tackle this if given a bit of guidance as to how you'd like to handle the way forward.

@olleolleolle
Copy link
Member Author

Hello @d-m-u and thanks for the offer!

Apologies for a late reply. This is a big topic, so I have taken my time preparing to write this. 🕚

Overview: 🗺️ The Faraday Middleware repository does not have the goal to grow, but to shrink. Any new middlewares are better created as their own projects, their own repository and release process. This reduces the maintenance burden on the Faraday team, to build the main software, and not this collection of middlewares.

I have talked about this with the other maintainers, and they also wish to be able to concentrate on the Faraday project. (👋 @iMacTia & friends.)

Idea: 💭 Perhaps the time has come for the YAML middleware to take wings, and have a new repository home, so that it can be deprecated to be removed in the next major version?

Doing that would free that library's author up to pick a support schedule (say, Ruby's own Ruby Maintenance Branches, or an even smaller surface of support). The search space of options above is perhaps too big for Faraday Middleware - for a new gem, it could be cut down a lot.

By creating a pattern to follow when creating a new Faraday middleware project, we help the wider Faraday community serve its own needs.

Offer: 👉 If you'd like to pick an implementation path and go forward with this, I would support you. How do you feel about all this?

@d-m-u
Copy link
Contributor

d-m-u commented Apr 28, 2020

This sounds great and I'm absolutely willing to try!

@olleolleolle
Copy link
Member Author

@d-m-u faraday-http is a just-released Faraday-hosted example of an Adapter (outside the core project).

It's an example of a non-core gem (not a middleware, but an adapter).

Perhaps interesting for you.

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

No branches or pull requests

2 participants