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

Preserve object key ordering when generating output #222

Closed
bradrydzewski opened this issue May 10, 2018 · 13 comments
Closed

Preserve object key ordering when generating output #222

bradrydzewski opened this issue May 10, 2018 · 13 comments

Comments

@bradrydzewski
Copy link

bradrydzewski commented May 10, 2018

👋 Hi there, similar to ksonnet, there is interest in the drone community to use jsonnet to simplify pipeline configuration files. Our community is building increasingly complex yaml files like this that could really benefit from jsonnet.

For reference, this is a simple example of a yaml pipeline configuration that defines two pipeline steps, executed sequentially:

pipeline:
  frontend:
    image: node
    commands:
      - npm install
      - npm test
  backend:
    image: golang
    commands:
      - go build
      - go test

Ideally a jsonnet representation would directly mirror the yaml structure, giving developers the ability to more easily migrate from yaml to jsonnet. For example:

{
  frontend: {
    image: "node",
    commands: [
      "npm install",
      "npm test"
     ]
  },
  backend: {
    image: "golang",
    commands: [
      "go build",
      "go test"
     ]
  }
}

The challenge we face is that jsonnet orders keys alphabetically, which means we cannot rely on the ordering of the pipeline steps. Changing our yaml pipeline structure to use a slice instead of a map is unfortunately not an option for us, since it would break a large number of our existing installations.

I was therefore wondering if the community would be open to (optionally) retaining order, perhaps in a manner similar to go-yaml. The go-yaml package implements custom types that retain order when unmarshaled and marshaled:

I do apologize if it seems forward to ask a go-jsonnet to add complexity to its implementation to support the a single project/community, and I certainly understand if such a change/customization is not desired. If there is interest, however, I would happily volunteer to author such a change. Thanks for the consideration!

@bradrydzewski bradrydzewski changed the title Preserve object key ordering Preserve object key ordering when generating output May 10, 2018
@bradrydzewski
Copy link
Author

I am closing this because it was brought to my attention that, like json, yaml does not guarantee key ordering and drone has been out of compliance this whole time. I guess I should have read the 80 page spec more closely 😛. Since this appears to be a non-trivial change to jsonnet and drone is out of compliance, this doesn't make sense. Sorry for the noise!

[1] http://yaml.org/spec/1.2/spec.html#id2765608

In the representation model, mapping keys do not have an order. To serialize a mapping, it is necessary to impose an ordering on its keys. This order is a serialization detail and should not be used when composing the representation graph (and hence for the preservation of application data). In every case where node order is significant, a sequence must be used. For example, an ordered mapping can be represented as a sequence of mappings, where each mapping is a single key: value pair. YAML provides convenient compact notation for this case.

@sbarzowski
Copy link
Collaborator

Hello @bradrydzewski,

We're always happy to hear about any ideas or needs 😄 . FYI it has been asked on at least 2 occasions before (in this issue google/jsonnet#407). This is one of these cases when there are good reasons to do it, but it also causes problems. Doing anything like that would require extensive discussion. An acceptable solution would probably require some boilerplate or special syntax. It shouldn't affect semantics very deeply - it would be just a hidden field or something similar under the hood.

You're right that since your output format doesn't preserve ordering, it doesn't make any sense to go further with it now. I hope you figure out a good solution for drone. Please, let us know if you have any other ideas!

@metalmatze
Copy link

Hey @sbarzowski

Thanks for taking the time to listen to our needs. Can you quickly point out your thoughts on this topic? For now I've simply gone ahead and added numbered prefixes to the steps to be executed in the pipeline. This obviously looks very strange: example.

Do you think there's a possibility of creating an array with persistent order within the jsonnet file and then later on converting it to a object with the order preserved?
I'm currently not really sure on what's the best way forward. Worst case scenario, we'd need to generate those number prefixes in a loop.

Thanks for your time!

@sbarzowski
Copy link
Collaborator

sbarzowski commented May 11, 2018

Sure.

EDIT: once I started thinking about it, it wasn't so quick, I produced quite an essay below :-). But no worries, it was fun.

numbered prefixes

It would be really annoying to add something in the middle. If you want numbered prefixes in the output, it's still better to get it as an array from the user and generate them in jsonnet.

Worst case scenario, we'd need to generate those number prefixes in a loop.

If you mean generating jsonnet code with these prefixes - please don't 😄. If you mean having a jsonnet function which takes an array and builds object with these prefixes for your tool to consume - that's an easy (<10 lines in jsonnet total) and very reasonable option if this is the output format you're going for.

Do you think there's a possibility of creating an array with persistent order within the jsonnet file and then later on converting it to a object with the order preserved?

Yes, definitely. You'll just need to wrap an array in a function, which will transform it to the target format, e.g. object with the order preserved, however you decide to represent it.

There are three questions:

  1. How do you want the input (i.e. jsonnet code defining the pipeline) to look like?
  2. How should it be represented internally?
  3. What should be the output format?

(1) is mostly about ergonomics. My first idea was to make everything a function call.

pipeline: drone.pipeline([
    drone.stepDef(
        'restore', 
        drone.step.s3Cache.new(restore=true, secrets=cacheSecrets)
    ),
    drone.stepDef(
         'composer',
        drone.step.new('owncloudci/php:${PHP_VERSION}', commands=['./tests/drone/composer-install.sh']
    ),
    // and so on
])

That's pretty heavy boilerplate, however. Alternatively instead of stepDef call, you can use a single-field object ({composer: drone.step.new(...}), or an array with two elements or perhaps just pass the name to new functions.

drone.pipeline transforms from whatever format is most convenient for input to (2).

One non-obvious thing to check is that your format should look nice after passing it through autoformatter - jsonnet fmt (you need old cpp jsonnet command for that, currently).

(2) Can be whatever you want really. May be an object with a hidden "fieldOrder" array to define order, it may be just an array, it may be an object with an array of fields and a separate object to map names to these fields... If you're not doing any real processing in jsonnet before (3), drone.pipeline could transform it directly to (3).

(3) It's a matter of what your tool is going to accept. You can output an array, which I think is the cleanest option, or you can generate numbered fields, or you can write a custom manifester which will output yaml with the fields in order.

@bradrydzewski
Copy link
Author

bradrydzewski commented May 11, 2018

FWIW prior to closing the issue I had put together a small patch that optionally retains ordering. I had envisioned this being at the library level (e.g. a setting with the interpreter / vm) and that drone would directly import jsonnet and incorporate in the drone CLI. For reference in case there is ever any interest, here is a gist with the diff (edit: here is the fork) master...drone:master

Most of the changes replace map[string]interface{} types with a structure that stores the map as well as the original field order in a slice.

Note that this patch requires cleanup and new unit tests (existing tests are passing) but could be a good baseline should you ever decide that preserved ordering could be of value.

@bradrydzewski
Copy link
Author

bradrydzewski commented May 11, 2018

@sbarzowski after discussing with @metalmatze, I think for now we are going to keep a fork with the ability to optionally preserve key order with a vm flag. You can see the changes here master...drone:master

Unfortunately this seems like the only viable option since we cannot change the .drone.yml file format, and we really want to provide jsonnet support to our users. Once we have jsonnet integrated into Drone, and if it becomes widely adopted by our community, I might re-visit this topic and the ability to merge my fork. But for now, this is mostly an experiment so it probably doesn't make sense yet to propose any upstream changes 😄

Thanks again for your comments above.

@sparkprime
Copy link
Contributor

What I would do is write a custom manifester that takes something of the form [ {key: value},... ] and will treat it specially, by outputting the keys in the order given instead of alphabetically.

Basically you need to output non-standard YAML / JSON, so you need a custom manifester. Since the internal object model isn't ordered (not clear how to preserve that in general with things like reflection, object composition, object comprehension), you need to represent the ordering within the object with something like a list of pairs instead of a Jsonnet object value.

@sparkprime
Copy link
Contributor

Alternatively, fix the consumer of the non-standard format to accept a array of pairs in lieu of an object.

@bradrydzewski
Copy link
Author

bradrydzewski commented May 11, 2018

@sparkprime first of all, thanks for taking the time to provide input. I really appreciate it, especially given that my problems are self-induced by having created a non-standard yaml format!

What I would do is write a custom manifester that takes something of the form [ {key: value},... ] and will treat it specially, by outputting the keys in the order given instead of alphabetically.

This is a great suggestion and the right solution from a technical perspective. The only reason I hesitated with approach is that I wanted to keep the same structure for the yaml and jsonnet file to simplify our documentation and support. It is easier for us to document an identical structure for both formats. I also anticipate our users will assume the exact same structure, which could lead to a spike in support requests if different structures were used.

Basically you need to output non-standard YAML / JSON, so you need a custom manifester

In the fork I created, I added a flag here that preserves key order in the json output. This raw json output can then be parsed by the go-yaml library with the order retained using our custom struct which implements yaml.Unmarshaler.

not clear how to preserve that in general with things like reflection, object composition, object comprehension

This is a great point. I'm new to jsonnet and it is possible that when alphabetical sorting is optionally disabled, it ends up breaking some features. This is probably something @metalmatze and I will have to test out further.

Alternatively, fix the consumer of the non-standard format to accept a array of pairs in lieu of an object.

I agree and think we (Drone) need to introduce a version: 2 yaml file format that is compliant with the yaml specification and does not rely on key ordering. In the short term, I'm hoping my fork provides us the ability to introduce jsonnet to the Drone community right away, and that we can drop my fork once we have a v2 yaml.

@zxy198717
Copy link

zxy198717 commented Feb 15, 2019

@bradrydzewski seems your fork has been deleted?

  • I want to use jsonnet to write swagger.
  • Generate api document based on swagger file.
  • I decide that preserved ordering could be of value for api document.

Can you restore your fork?

Thanks for your time!

@JohnRusk
Copy link

I would also like preservation of order. (For the human consumers of the file, not for machines which should, correctly, be order agnostic).

@kristijorgji
Copy link

I started to use jsonnet and this is a BIG ISSUE!
Not something small, because any templating language should respect the template layout and order.

Saying this, any update on this or rfc or is not possible ?

@sbarzowski
Copy link
Collaborator

We're tracking this in google/jsonnet#407. I'm open to proposals here if someone wants to pick this up. Stuff that needs to be considered includes (a) how is it going to work with inheritance (b) how is it going to work with object comprehensions (c) make sure referential transparency is preserved (d) it should be possible to override the order for known formats (so that you can normalize outputs coming from various places, so that the output is always in the same, preferred order).

There was also some discussion in the mailing list about this.

This is a language change, not anything specific to the interpreter implementation.

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

7 participants