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

Yaml plugin addition #82

Merged
merged 14 commits into from
Mar 3, 2023
Merged

Yaml plugin addition #82

merged 14 commits into from
Mar 3, 2023

Conversation

JakeMHughes
Copy link
Contributor

This PR adds the YAML plugin which can be accessed using application/yaml Since yaml is becoming used in more & more configuration files. This would be useful for validating and transforming said configurations (OASv3 and K8s are good examples) .

This plugin supports Reading & writing both single Yaml documents as well as multiple documents. i.e.

---
message: test
---
test: Hello World

If only a single document is passed, you access the contents like its a json object, like: payload.message. Single documents also have the option to disable the yaml head( the --- line) with the option RemoveHead=true

If a multi document is passed, you access the contents like its a json array, like: payload[0].message

Both types have access to the DisableQuotes=true header which will remove quotes from the strings

@JakeMHughes JakeMHughes requested review from fugu13 and jam01 December 3, 2020 18:53
@jam01
Copy link
Contributor

jam01 commented Jan 23, 2021

Good stuff, I think this is actually very valuable and we could use it in our kube work. The changes requested are minimal.

A bigger question we need to answer is if it should actually work exactly as the json plug-in, same parameters, same behavior, bean (un)marshalling, etc. Or do we pick and choose? Because ultimately yaml is a superset of json

src/main/java/com/datasonnet/document/MediaTypes.java Outdated Show resolved Hide resolved

try {
Object inputAsJava = ujsonUtils.javaObjectFrom(input);
ObjectMapper yamlMapper = new ObjectMapper(new YAMLFactory());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed this one to ObjectMapper yamlMapper = new ObjectMapper(DEFAULT_YAML_FACTORY); not sure if you wanted a third constant for an object mapper initialized with the yaml factory

StringBuilder value = null;

//if instance of list, it is multiple docs in one.
if(inputAsJava instanceof List){
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'll need a writer parameter to tell us what to do here. Something like arrayAsMultiDoc

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What would we do instead in this case?
im assuming if arrayAsMultiDoc is false it would do something else, but what else could it do while still being valid yaml?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would come out as a single doc that's an array in YAML, those are legal top-levels, just like in JSON (some JSON processors restrict that, but the spec definitely allows it)


public class DefaultYamlFormatPlugin extends BaseJacksonDataFormatPlugin {

public static final String DS_PARAM_YAML_HEADER = "removehead";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is header the official / accurate term in yaml?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not really, it doesnt have a name for it: https://yaml.org/spec/1.1/#id857577

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you click the three dashes, they have a name in that section, Document Boundary Markers

@fugu13
Copy link
Contributor

fugu13 commented Jul 2, 2021

@JakeMHughes I made a couple comments (not in a review) above, but agreed with @jam01 this is good stuff we want to include once we've fixed a few things

@javaduke javaduke merged commit a104f82 into main Mar 3, 2023
@javaduke javaduke deleted the yaml-plugin branch March 3, 2023 18:00
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