Skip to content

Conversation

@brian-lou
Copy link

No description provided.

@asottile-sentry
Copy link
Contributor

I would strongly suggest not using this library -- its maintenance is shaky at best and done in the dark on sourceforge.

I know my name is in the README of it but having used it a bunch it barely does what it says on the tin ("roundtrip yaml") and is a bigger liability than a feature

what were you planning on using it for?

@brian-lou
Copy link
Author

brian-lou commented Oct 29, 2024

I would strongly suggest not using this library -- its maintenance is shaky at best and done in the dark on sourceforge.

I know my name is in the README of it but having used it a bunch it barely does what it says on the tin ("roundtrip yaml") and is a bigger liability than a feature

what were you planning on using it for?

We are planning to use it in sentry-infra-tools for quickpatch: https://www.notion.so/sentry/Sentry-quick-patch-aka-Instant-Incident-Generator-1298b10e4b5d80ccad7ffa7403269cd7

We need a yaml library that:

  1. Can parse yaml files into a dict while maintaining comments & other non pure-yaml things
  2. Can save the dict back into a .yaml file (and keep the comments) from before
  3. Is a python package so that we don't have to expect users to already have the library pre-installed

From what we found, this was the library that best suited those needs. Things like yq or cels didn't quite fit the criteria
cc: @fpacifici

@asottile-sentry
Copy link
Contributor

I would strongly suggest not using this library -- its maintenance is shaky at best and done in the dark on sourceforge.
I know my name is in the README of it but having used it a bunch it barely does what it says on the tin ("roundtrip yaml") and is a bigger liability than a feature
what were you planning on using it for?

We are planning to use it in sentry-infra-tools for quickpatch: https://www.notion.so/sentry/Sentry-quick-patch-aka-Instant-Incident-Generator-1298b10e4b5d80ccad7ffa7403269cd7

We need a yaml library that:

  1. Can parse yaml files into a dict while maintaining comments & other non pure-yaml things
  2. Can save the dict back into a .yaml file (and keep the comments) from before
  3. Is a python package so that we don't have to expect users to already have the library pre-installed

From what we found, this was the library that best suited those needs. Things like yq or cels didn't quite fit the criteria cc: @fpacifici

unfortunately I don't think ruamel.yaml will fit that either -- it simply does not preserve yaml properly in many many cases

how much of the yaml is changing? if you're targetting specific scalars it's doable with pyyaml's tokenization -- if it's more complicated though I'm afraid there's no good solution

@fpacifici
Copy link
Contributor

fpacifici commented Oct 30, 2024

From what we found, this was the library that best suited those needs. Things like yq or cels didn't quite fit the criteria.

We should add that we went through a thorough and careful investigation .... that took 11 minutes on a google meet. So I guess we can look for better options. Even if we cannot preserve comments, we may be able to live with that.

@brian-lou Please keep working on the patch system using pyyaml and ignoring comments. I will look for a better library this afternoon

@asottile-sentry may I aask you a few more details on "it simply does not preserve yaml properly in many many cases" do you have negative past experience with that or should we just look around in forums to find examples ? So we can exclude it with stronger reasons and avoid making the same mistake in the future.

@asottile-sentry
Copy link
Contributor

From what we found, this was the library that best suited those needs. Things like yq or cels didn't quite fit the criteria.

We should add that we went through a thorough and careful investigation .... that took 11 minutes on a google meet. So I guess we can look for better options. Even if we cannot preserve comments, we may be able to live with that.

@brian-lou Please keep working on the patch system using pyyaml and ignoring comments. I will look for a better library this afternoon

@asottile-sentry may I aask you a few more details on "it simply does not preserve yaml properly in many many cases" do you have negative past experience with that or should we just look around in forums to find examples ? So we can exclude it with stronger reasons and avoid making the same mistake in the future.

I can dig up some examples but I've basically spent the last decade trying to solve this problem ~somewhat unsuccessfully for pre-commit migrate-config and pre-commit autoupdate -- my solutions so far are utilize the token stream or regular expressions -- the former is at least "sound" but somewhat complicated (and mostly limited to scalar replacements).

the core problem is ruamel.yaml isn't a CST -- it instead attaches comments to "nearest nodes" and does not preserve flow style or related whitespace in most cases (and especially not after a rewrite -- sometimes merging the comments into unrelated nodes or into values!)

@fpacifici
Copy link
Contributor

ok, thanks for the info.

@brian-lou please let's close this PR

@brian-lou brian-lou closed this Oct 30, 2024
@asottile-sentry asottile-sentry deleted the brian/add-ruamel branch November 12, 2024 19:16
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 this pull request may close these issues.

4 participants