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

[Question] Configurations with many optional fields #35

Closed
timbod7 opened this Issue Apr 13, 2017 · 8 comments

Comments

Projects
None yet
2 participants
@timbod7

timbod7 commented Apr 13, 2017

I'm looking at Dhall for the first time, and I like what I see so far.

When building systems the configuration for a component may have a small number of required fields, and a larger number of fields that have sensible defaults, but may be overriden as required. For an example, consider the terraform specfication for an AWS EC2 instance. There are 24 configurable fields, some of which must be provided, the others have useful defaults.

How would this be expressed in dhall? The straighforward translation would seem to to use the Optional builtin, ie:

$ cat configEc2
\(params : {
    ami : Text,
    availability_zone : Optional Text,
    placement_group : Optional Text,
    instance_type : Text
  }) -> "...build some structure out of the parameters, filling in the defaults..."

But the issue with this is that I have to provide all of the optional fields, along with their types, ie:

$ cat test3
./configEc2 {
  ami = "ami-12345",
  availability_zone = [] : Optional Text,
  placement_group = [] : Optional Text,
  instance_type = "t2.small"
}

With many optional values, this is verbose and tedious. It also doesn't allow optional values to be added without updating all of the call sites. Is there a means by which I only have to name the required fields, along with any options that I wish to override?

@Gabriel439

This comment has been minimized.

Show comment
Hide comment
@Gabriel439

Gabriel439 Apr 13, 2017

Collaborator

I think probably the most straightforward solution is to change the record merge operator (i.e. /\ or ) to tolerate field collisions and pick the field from the right record if there is a collision. This then lets you use it as an associative, right-biased, field update operator.

Then you could provide the user with a default record with all optional values set to empty and let them selectively override them. For example, you could do:

$ cat defaults
{ availability_zone = [] : Optional Text
, placement_group = [] : Optional Text
}

... and then build up the final record like this:

  ./defaults
∧ { ami = "ami-12345"
  , instance_type = "t2.small"
  , placement_group = [ "Foo" ] : Optional Text
  }

... if you wanted to override the placement_group default value but leave availability_zone as the default (i.e. unset).

How does that sound?

Collaborator

Gabriel439 commented Apr 13, 2017

I think probably the most straightforward solution is to change the record merge operator (i.e. /\ or ) to tolerate field collisions and pick the field from the right record if there is a collision. This then lets you use it as an associative, right-biased, field update operator.

Then you could provide the user with a default record with all optional values set to empty and let them selectively override them. For example, you could do:

$ cat defaults
{ availability_zone = [] : Optional Text
, placement_group = [] : Optional Text
}

... and then build up the final record like this:

  ./defaults
∧ { ami = "ami-12345"
  , instance_type = "t2.small"
  , placement_group = [ "Foo" ] : Optional Text
  }

... if you wanted to override the placement_group default value but leave availability_zone as the default (i.e. unset).

How does that sound?

@timbod7

This comment has been minimized.

Show comment
Hide comment
@timbod7

timbod7 Apr 13, 2017

That sounds pretty good. In fact the behaviour you are describing is what I expected that the merge operator would do (until I read the docs!)

It still means the for each resource I want to create, I need to expose two values: a function to actually create the resource (xfn), and a record of default values for the that function (xdefaults), with each call site looking like:

xfn (xdefaults /\ {
  ... my named params go here ...
  })

It's probably ok, and definitely cleaner than my current haskell EDSL. I guess I pine for python's named function argument passing, ie:

def configEc2(
    ami,
    instanceType,
    availability_zone = None,
    placement_group = None
    ):

Of course, there's a whole bunch of reasons why I don't want to use python (typing for a start), but I do like the argument passing :-)

timbod7 commented Apr 13, 2017

That sounds pretty good. In fact the behaviour you are describing is what I expected that the merge operator would do (until I read the docs!)

It still means the for each resource I want to create, I need to expose two values: a function to actually create the resource (xfn), and a record of default values for the that function (xdefaults), with each call site looking like:

xfn (xdefaults /\ {
  ... my named params go here ...
  })

It's probably ok, and definitely cleaner than my current haskell EDSL. I guess I pine for python's named function argument passing, ie:

def configEc2(
    ami,
    instanceType,
    availability_zone = None,
    placement_group = None
    ):

Of course, there's a whole bunch of reasons why I don't want to use python (typing for a start), but I do like the argument passing :-)

@Gabriel439

This comment has been minimized.

Show comment
Hide comment
@Gabriel439

Gabriel439 Apr 13, 2017

Collaborator

So I may need to introduce a separate operator for this purpose, mainly due to the fact that the operator's behavior of recursively merging records does not play nicely with tolerating field collisions:

  • handling this correctly requires interleave type-checking with normalization
  • giving consistent behavior requires also permitting to work on non-record values (i.e. 1 ∧ "Foo" would type-check and yield "Foo" as the result)

So if nobody objects what I think I will do instead is introduce a new right-biased non-recursive merge operator for this purpose. I might name the operator// (same as Nix) with ⫽ (U+2AFD) being the Unicode version. This would mean that you wouldn't be able to easily supply nested default values but given that the purpose of this is to emulate something like Python's default function arguments I imagine that shouldn't be a problem.

Collaborator

Gabriel439 commented Apr 13, 2017

So I may need to introduce a separate operator for this purpose, mainly due to the fact that the operator's behavior of recursively merging records does not play nicely with tolerating field collisions:

  • handling this correctly requires interleave type-checking with normalization
  • giving consistent behavior requires also permitting to work on non-record values (i.e. 1 ∧ "Foo" would type-check and yield "Foo" as the result)

So if nobody objects what I think I will do instead is introduce a new right-biased non-recursive merge operator for this purpose. I might name the operator// (same as Nix) with ⫽ (U+2AFD) being the Unicode version. This would mean that you wouldn't be able to easily supply nested default values but given that the purpose of this is to emulate something like Python's default function arguments I imagine that shouldn't be a problem.

Gabriel439 added a commit that referenced this issue Apr 13, 2017

Add `(//)` for right-biased, non-recursive merge. Fixes #35
The expected use of this operator is to update a record.  For example,
you can provide a record with default values, like this:

```bash
$ cat defaults
{ availabilityZone = [] : Optional Text
, placementGroup   = [] : Optional Text
}
```

... and then update or extend that record with new values using `(//)`:

```bash
$ dhall
./defaults //
{ ami = "ami-12345" -- Example of adding a new field
, availabilityZone = ["eu-west-1"] : Optional Text -- Overriding field
-- Use the default for placementGroup
}
<Ctrl-D>
{ ami : Text, availabilityZone : Optional Text, placementGroup
: Optional Text }

{ ami = "ami-12345", availabilityZone = ["eu-west-1"] : Optional Text, placementGroup = [] : Optional Text } ```
```

You can also override a field with a new type as well:

```bash
$ dhall
{ foo = 1 } // { foo = "ABC" }
<Ctrl-D>
{ foo : Text }

{ foo = "ABC" }
```
@Gabriel439

This comment has been minimized.

Show comment
Hide comment
@Gabriel439

Gabriel439 Apr 13, 2017

Collaborator

I have a pull request up for the proposed change here: #36

Collaborator

Gabriel439 commented Apr 13, 2017

I have a pull request up for the proposed change here: #36

@timbod7

This comment has been minimized.

Show comment
Hide comment
@timbod7

timbod7 Apr 14, 2017

Nice!

The change looks great. The example is written with defaults as Optional values, but this would work with any types, wouldn't it?

timbod7 commented Apr 14, 2017

Nice!

The change looks great. The example is written with defaults as Optional values, but this would work with any types, wouldn't it?

@Gabriel439

This comment has been minimized.

Show comment
Hide comment
@Gabriel439

Gabriel439 Apr 14, 2017

Collaborator

Yes, this would work with any type, not just Optional. The value that you override with can even be a different type from the original value (although I'm not sure what use that would be)

Collaborator

Gabriel439 commented Apr 14, 2017

Yes, this would work with any type, not just Optional. The value that you override with can even be a different type from the original value (although I'm not sure what use that would be)

@timbod7

This comment has been minimized.

Show comment
Hide comment
@timbod7

timbod7 commented Apr 14, 2017

Thanks!

@Gabriel439

This comment has been minimized.

Show comment
Hide comment
@Gabriel439

Gabriel439 Apr 14, 2017

Collaborator

You're welcome! :)

Collaborator

Gabriel439 commented Apr 14, 2017

You're welcome! :)

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