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

Syntax for optional array concatenation #234

Open
huggsboson opened this issue Aug 31, 2016 · 13 comments
Open

Syntax for optional array concatenation #234

huggsboson opened this issue Aug 31, 2016 · 13 comments

Comments

@huggsboson
Copy link
Contributor

At Box we've often had a need to optionally include elements in an array, e.g.

util.list([
        namespace.object(),
        deployment.object(logtailer),
        service.object(),
    ] + logtailer.configMaps
      + if cluster.isDSV31 then [ logback.configMap() ] else []
)

While the current approach is effective it has two downsides:

  1. It isn't super readable.
  2. The tendency is to prepend or append the items so you don't have to break up the list to insert an array.

Either some guidance on how best to handle cases like this, or an extension to the syntax would be nice. I talked to @sparkprime yesterday in slack and he spun up a suggestion to talk about:

util.list([
        namespace.object(),
        configMap for configMap in logtailer.configMaps,
        logback.configMap() if cluster.isDSV31,
        deployment.object(logtailer),
        service.object(),
])

Which is much more readable and deals with #1 and #2.

We began talking about potential implementations for this, and explored the idea of an undefined type/value which stays in the list until render time (and comparison) at which point it is removed. That idea has a lot of merit and would even allow functions to return undefined thereby pushing certain branching logic down, but there are obviously a lot of interesting corner cases.

Dave also mentioned that if such a proposal went through he'd probably want to move from
{ [if false then "foo"]: "bar" } to { foo: "bar" if false }.

@sparkprime
Copy link
Contributor

@chrisleck had similar feedback in the context of maps and the same solution should be used for both cases

@chris-codaio
Copy link

+1 to some form of undefined or null that is removed at render time in arrays/maps. I just ran into this problem last week in an array definition where one of the elements was optional based on an optional param to a function generating it. Having the optional param default to "None" or "Null" or "undefined" and then not having it show up in the subsequent array would be magical.

@mikedanese
Copy link
Contributor

I would like this. For reference, here's a snippet that we use to work around not having this for the object case.

function(cfg)
  local if_enabled(addon, manifest) = if cfg.phase3[addon] then manifest else {};
  local join(arr) = std.foldl(function(a, b) a + b, arr, {});
  if_enabled("run_addons",
             join([
               if_enabled("kube_proxy", (import "kube-proxy/kube-proxy.jsonnet")(cfg)),
               if_enabled("dashboard", (import "dashboard/dashboard.jsonnet")(cfg)),
               if_enabled("heapster", (import "heapster/heapster.jsonnet")(cfg)),
               if_enabled("kube_dns", (import "kube-dns/kube-dns.jsonnet")(cfg)),
             ]))

From here

@huggsboson
Copy link
Contributor Author

We ended up adding a utility function:

    // This is really useful if you want to make an arry out of
    // constitutent parts which may be lists or optional.
    //
    // Returns the passed array with:
    // 1. Nulls removed
    // 2. Any elements who are arrays flattened into this arry.
    local join(a) =
        local notNull(i) = i != null;
        local maybeFlatten(acc, i) = if std.type(i) == "array" then acc + i else acc + [i];
        std.foldl(maybeFlatten, std.filter(notNull, a), []);

Which lets us do both
join([
if true then x,
if false then y,
[listOfObjs],
])

and serves most of our use cases. I'm gonna close this for now.

@sparkprime
Copy link
Contributor

Re-opening because there ought to be a better solution for this

@sparkprime sparkprime reopened this Jan 30, 2017
@nikolay
Copy link

nikolay commented Jan 11, 2018

@sparkprime I suggested in #ksonnet on Slack adding an unambiguous type of empty, which is different than null.

Adding empty to an array would not do anything. Adding a key with an associated value of empty to an object would also not do anything - maybe it should do the same if the key itself is empty, too. Appending empty to a string would not do anything either (it would be equivalent to an empty string), and as a number - equal to zero, - but maybe only when adding or subtracting.

I also suggested borrowing from Ruby and adding infix if and unless, which would return empty instead of null when there's no else value. I also suggested adding a similar switch or case function.

Similarly, in method chains (something.withProp1(...).withProp2(...).withProp3(...)), it would also be ideal to be able do something.withProp1(...).(withProp2(...) if condition2).(withProp3(...) if condition3), etc.

Although empty and conditional function chains may seem unrelated, they both aim to make dynamic composition easier. std.prune() often removes valid values like null, and empty arrays, and objects, where empty will be completely unambiguous given it does not exist in JSON.

@sparkprime
Copy link
Contributor

My preferred way of doing this is to extend the python composition syntax to allow things like

[ 1, 2 if true, 3 if false ] == [ 1, 2 ]

I.e., rather than having 2 kinds of object literal (composition and non-composition) we instead have a single one that encompasses everything and squares it all out. You're allowed to put a "forspec" on any fields.

I agree std.prune isn't the best way to do this, it was just easy.

@nikolay
Copy link

nikolay commented Feb 9, 2018

@sparkprime Please borrow unless from Ruby, too!

@bernadinm
Copy link

bernadinm commented Oct 8, 2019

This function removes empty values in a list as also used in the custom join(a) shared previously.

std.prune(x)
Recursively remove all "empty" members of `x`. "Empty" is defined as zero length `arrays`, zero length `objects`, or `null` values.

https://jsonnet.org/ref/stdlib.html

@jacksgt
Copy link

jacksgt commented May 9, 2020

Hi, I just wanted to chime in and mention another use-case for an empty element.

I'm using Jsonnet to generate AWS IAM policies. The basic setup looks like this:

{
  Version: '2012-10-17',
  Statement: [
    {
      Effect: 'Allow',
      Action: [
        'cloudformation:List*',
        'cloudformation:Get*',
        'cloudformation:ValidateTemplate'
      ]
      Resource: '*'
    },
    {...}
  ]
}

If I want to add more policy statements based on conditions, I would (intuitively) do it so:

local s3Access = true;
{
  Version: '2012-10-17',
  Statement: [
    {
      Effect: 'Allow',
      Action: [
        'cloudformation:List*',
        'cloudformation:Get*',
        'cloudformation:ValidateTemplate'
      ]
      Resource: '*'
    },
    if s3Access {
      Effect: 'Allow',
      Action: [
        's3:GetBucketLocation',
        's3:CreateBucket',
        's3:ListBucket',
        's3:ListBucketVersions'
      ]
      Resource: '*'
     }
  ]
}

However, in case that s3Access = false, this will produce null as the last element, which AWS IAM refuses as invalid.

Currently, I'm using the following workaround:

local s3Access = true;
policy = {
  Version: '2012-10-17',
  Statement: [
    {
      Effect: 'Allow',
      Action: [
        'cloudformation:List*',
        'cloudformation:Get*',
        'cloudformation:ValidateTemplate'
      ]
      Resource: '*'
    },
    if s3Access {
      Effect: 'Allow',
      Action: [
        's3:GetBucketLocation',
        's3:CreateBucket',
        's3:ListBucket',
        's3:ListBucketVersions'
      ]
      Resource: '*'
     }
  ]
};

// need to remove all empty / null elements from the document
std.prune(policy)

Having an empty element (or similar) would make this case much more readable.

@mhemken-nyt
Copy link

Hello from 2021.
I needed optional array elements but the syntax is not super straightforward. I ended up doing something like this:

local utils = {
  StringArr1:: [ ... ],
  StringArr2:: [ ... ],
  StringArr3:: [ ... ],
  AllStrings(condition1=false, condition2=false)::
    [x for x in utils.StringArr1]
    + [x for x in utils.StringArr2 if condition1]
    + [x for x in utils.StringArr3 if condition2],
};

Is there a cleaner way of doing this yet?

@sbarzowski
Copy link
Collaborator

Yeah, e.g.:

local arrayIf(arr, condition) = if condition then array else [];

local utils = {
  StringArr1:: [ ... ],
  StringArr2:: [ ... ],
  StringArr3:: [ ... ],
  AllStrings(condition1=false, condition2=false)::
    utils.StringArr1
    + arrayIf(utils.StringArr2, condition1)
    + arrayIf(utils.StringArr3, condition2),
};

@huggsboson
Copy link
Contributor Author

We've had pretty good luck with the aforementioned join() method. We've been using it quite extensively for quite a while.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

9 participants