Release of 4.0.0 breaks yaml files with dates #489
Comments
|
4.0 introduces a set of new methods, #unsafe_load_file is among them. |
|
@olleolleolle sure it may have new methods but the underlying changes are breaking to existing |
|
@tenderlove I do not believe that is acceptable. I have taken my project entirely out of the picture by running the following: In updating the Gemfile to lock psych to 3.x, place the following file in the ---
development:
date: 2013-10-16And execute the following Lock in psych-4.x and perform the same execution, I get Something is terribly wrong with your rubygem and the 4.0 version should be pulled from rubygems. |
|
@tenderlove if such a change was deemed necessary, deprecation notices should have been provided directing consumers that 'hey, a change is coming you may be concerned about' and not just breaking people on an upgrade. If you wish to continue with the existing logic, you should provide fallback to original if an exception occurs and provide a deprecation notice to again indicate to your consumers that they will need to change their way of loading yaml files. |
|
@poloka It's a major version change. You can choose whether or not to upgrade, and if you'd like to add those kind of warnings to the 3.x series, I'm happy to merge your pull requests! 😄 |
|
Semantic versioning dictates that a major version change happens when breaking changes are introduced:
3.x.y to 4.x.y is a MAJOR version change, and you should expect breaking changes. As Aaron points out, you can limit your project to |
|
@JonRowe I am very aware of semantic versioning practices but I also understand the responsibility of a developer to provide warnings of such breaking changes and documentation of said changes when they do occur. Neither has happened in regards to this breaking change. And yes, I will lock in the version to a 3.x until I have determined a way to remove psych from my tech stack. |
|
@poloka I'm sure the maintainers would be happy to refund all of the money you gave them to use this gem |
|
Maybe I should phrase it more like this, it is a 'courtesy' to warn our consumers and if we know there is a breaking change that we provide migration guides to assist in the pains of pulling the rug out from under them. I'm just trying to follow best practices to my/our consumers in order to keep them productive and keep disruptions to a minimum. Just trying to give my $0.02 on the topic. |
That is to say, none, a major version change is the warning. Please lock your major versions if you are unable to cope with breaking changes. :) |
|
@poloka Did you mean that "our consumers" are your project or product's consumers using psych as one of the dependencies? I am sorry about your situation. However a breaking change can happen for every gem as a possibility. So, I think the best practice you can learn for your project or product from this situation is to pin the dependency gem versions as much as possible such as with |
|
@JonRowe we ran into psych being a transitive dependency from another project so we didn't have knowledge of its version or had initial control over the version being taken. So we were blissfully unaware of the open-ended accepted versions. Thanks @junaruga , yeah we utilize the BTW, thanks for making it a MVB. I've run into other gems where breaking changes were in minor versions. Talk about a pain. Thanks guys for the direction! |
- `YAML.load_file` now fails because the library authors added a security check that doesn't allow regexes to be parsed. The old behaviour is now behind `YAML.unsafe_load_file` ruby/psych#489 - Update Alpine to 3.15
|
I ran into this while upgrading to Ruby 3.1 and I do feel that the messaging on this breaking change could be improved. The Ruby 3.1.0 release notes list the breaking change as basically a footnote in the "Other changes" section and don't provide a direct migration recommendation, they just link to an issue with discussion on whether Psych 4 should be included in Ruby 3.1 or not. When going to the official documentation or to this repo I can't find any official changelog or guide on what exactly changed. The releases tab on this repo also doesn't have a changelog for 4.0.0. It seems to me like Pull Request 487 is the most official looking warning/guide/documentation on what changed but it's not prominently linked from the obvious places, so I think a lot of people will waste some time trying to find the relevant information about this change, like I did. To be clear: I agree with the change and I'm thankful for the work done here, I'm just giving some (hopefully constructive) feedback on how the change was communicated. |
There is breaking change for Psych(=YAML). shouhizei cut the support for Ruby 2.7 refs: https://www.docswell.com/s/pink_bangbi/K67RV5-2022-01-06-201330#p19 ruby/psych#489
There is breaking change for Psych(=YAML). shouhizei cut the support for Ruby 2.7. This PR update `load_file` to `safe_load_file` with `permitted_classes` option. refs: https://www.docswell.com/s/pink_bangbi/K67RV5-2022-01-06-201330#p19 ruby/psych#489
|
UPDATE: Leaving this here in case anyone else has this issue. It turns out there is a goldilocks version of psych if you're having these issues. Anything before v3.1.0 won't let you pass permitted_classes as a method argument or a config argument in rails. Anything greater than ~v4 causes a I followed the advice for rails users (rails 5.2.8.1 in our case), and set Currently the only thing that works is Doesn't matter if I call Psych directly passing in the permitted classes: What do we do in this situation then. Shouldn't psych be reading our config change and allowing BigDecimals? EDIT: After digging into the gem (says version 2.2.2 inside versions.rb), I think there could be something wrong. When I run this with permitted classes, the @classes variable is not set correctly: |
With the release of 4.0.0, receiving the following error for yaml files with dates
Yaml file that I am attempting to load is
I was able to reproduce locally between two project by simply running the following code
The version with a 3.3.2 loads as expected
The text was updated successfully, but these errors were encountered: