Comments (15)
I was thinking something along the line of:
{
module_a = {
inputs | not_exported = {
name
| String,
},
module_a_name = inputs.name
},
module_b = {
inputs | not_exported = {
name
| String,
},
module_b_name = inputs.name
},
remove_inputs = fun r => r |> std.record.map_values std.function.id |> std.record.remove "inputs",
stack | not_exported = {
my_module_a =
module_a
& {
inputs = {
name = "my_module_a",
}
},
my_module_b =
module_b
& {
inputs = {
name = "my_module_b",
}
}
},
config =
std.record.values
stack
|> std.array.map remove_inputs
|> std.record.merge_all
}
In essence, remove_inputs
forces all recursive field dependencies to be resolved, and then gets rid of the inputs
field. This also means that you won't reasonably be able to override values using merging anymore, after applying remove_inputs
. Also, the std.record.map_values
trick is arcane, si I'd really like to come up with a better way of doing these things eventually.
from nickel.
Since the behavior of nickel==1.3.0
seems to be the "expected" one, I am going to close this issue. Feel free to reopen it if necessary.
Summary for everyone, who will hit this issue in future. We ended up with solution similar to @yannham proposal. Right now, we extract only a specific subset of fields from the module and so we are no longer facing the inputs
merge "issue".
Many thanks to @yannham and @vkleen!
from nickel.
I think the root cause is a difference in whether the inputs
field in config
gets evaluated or not. I suppose that as of Nickel 1.3, it gets evaluated even if it's not exported and evaluation can't succeed because of the different inputs
records in module_a
and module_b
. I think the behavior in Nickel 1.3 is somewhat reasonable, although I'm not sure why inputs
is forced...
from nickel.
A workaround, and probably an avenue for cleaner semantics here, would be to introduce an evaluation phare distinction: first process all overrides to a module, then remove the inputs
field using std.record.remove
and then merge the entire stack. Without something like that there won't be any way to distinguish which modules inputs
field is getting targeted, I think.
from nickel.
I think, I understand the idea, but I have no clue how to process all overrides to a module. Can you please bit elaborate on that?
from nickel.
Got it, many thanks for this workaround! It works reasonably for my case, since I don't need to override values any more.
I agree that it would be nice to have a better support for this "module like" semantic (without this ugly early evaluation hack).
from nickel.
@vkleen Isn't r |> std.record.map_values std.function.id
just r
, or am I missing something?
from nickel.
The contract on std.record.map_values
destroys the recursive thunks in r
. Without it, you'll get
error: unbound identifier `inputs`
┌─ /home/vkleen/work/tweag/nickel/nickel/master/test.ncl:14:21
│
14 │ module_b_name = inputs.name
│ ^^^^^^ this identifier is unbound
from nickel.
@uhlajs I would personally take a different approach: in some sense you have a namespace issue. In the merging model, it's a bit more annoying to handle. Here is the thing:
If all your modules share their inputs
field, because you merge them, then any parameter appearing in this inputs
field is "shared" and should be the same for all modules. Think of an environment of some sort.
Here, you use name
as a local module parameter, which is different for each module. One solution is to namespace it: for example, either use name_module_a
or module_a.name
or a name that is not the same as for module_b
.
Another solution would be to have a different field for such local inputs, such as locals
or local_inputs
or whatever. You could then remove it as @vkleen is doing. Or not merging it in the first place and just merge the config
field of each module (but in this case, you can't have a "global" shared inputs
anymore):
config =
stack
|> std.record.values
|> std.record.get "config" # Introduced in 1.4, it's just fun field r => r."%{field}"
|> std.array.map remove_inputs
|> std.record.merge_all
from nickel.
The contract on std.record.map_values destroys the recursive thunks in r.
Oh, interesting. So record.remove
doesn't freeze the record. Which is not entirely unreasonable, but can be surprising. Maybe it's time to add record.freeze
or record.fix
to do that
from nickel.
@yannham I was playing a bit with your suggestions (thanks for them!) and here is one "counter example" for the namespacing of the local module parameter. Let's consider slightly different example. I have a module, which I want to use twice with different input values.
{
module | not_exported = {
inputs | not_exported = {
name
| String,
},
resource."%{inputs.name}" = {},
config | not_exported = {
resource."%{inputs.name}" = {}
}
},
stack | not_exported = {
my_module_a =
module
& {
inputs = {
name = "my_module_a",
}
},
my_module_b =
module
& {
inputs = {
name = "my_module_b",
}
},
},
# Obviously doesn't work
# config = stack.my_module_a & stack.my_module_b,
remove_inputs | not_exported = fun r => r |> std.record.map_values std.function.id |> std.record.remove "inputs",
std_record_get | not_exported = fun field r => r."%{field}",
config_vkleen =
stack
|> std.record.values
|> std.array.map remove_inputs
|> std.record.merge_all,
config =
stack
|> std.record.values
|> std.array.map (std_record_get "config")
|> std.record.merge_all
}
Now, I cannot change the input parameter name, since the module is the same. On the other hand, the alternative solution with taking only module.config
works nicely. Actually, not having a "global" shared inputs is quite desirable here.
from nickel.
Ah, yes, this approach doesn't work if you have several copies of the same module. By nature, merging combines pieces together in one final value and I think it's hard to avoid that everything lives in the same namespace, at least when just using bare merging (though you could use contracts to restrict access probably).
The thing is, in the blog post, I argue that using record merging is more adapted than functions because it's a flat structure, it's easy to override and thus to reconfigure by relying on Nickel's merge system, and easy to combine.
If you just want to instantiate some parameter differently and that you are dead sure you don't want to access this parameter from the outer world (say, another module) or override it after the fact (you or the any other consumer of your code), I start to wonder if a simple plain function would actually do the trick.
That is, defining module_a = fun name => {...module def without inputs and using name instead of inputs.name...]
. Or, a way to see this would be a function that generates a concrete module from a parameter. In that case , name would probably need to be unique (each time it is provided) and you lose the ability to override it, but depending on your use-case, it might be the good tradeoff. Modules are but not everything has to be a module.
All of that being said,
- I'm still clueless as to why this code doesn't fail in 1.2.2. I think, as Viktor, that it's reasonable that it fails in 1.3.0, but I after a quick look I couldn't spot on obvious change in serialization or the handling of
not_exported
. Maybe some obscure recursive record bug fixing changed that. - Whether functions are the right choice for your particular use-case, it's still an interesting problem in general to think about those local variables, name-spacing issues, and "generative" modules. If I'm not mistaken, in the NixOS module system, they take the same "only consider
module.config
" route, so that when defining modules you can cross-refer to other module inputs (with e.g.module_a.inputs = ...
from withinmodule_b.config
), but onlyconfig
fields are combined in the end to form the final result.
from nickel.
What about this simple workaround
let module_a = {
inputs | not_exported = {
name | String,
},
output.module_a_name = inputs.name
}
in
let module_b = {
inputs | not_exported = {
name | String,
},
output.module_b_name = inputs.name
}
in
{
stack | not_exported = {
my_module_a = (module_a & {
inputs.name = "my_module_a",
}).output,
my_module_b = (module_b & {
inputs.name = "my_module_b",
}).output
},
config = stack.my_module_a & stack.my_module_b
}
from nickel.
@smamessier Your workaround is basically identical to the @yannham alternative proposal:
Or not merging it in the first place and just merge the config field of each module (but in this case, you can't have a "global" shared inputs anymore)
Following snippet just removes the complexity from the module field assignment to the config field assignment:
config =
stack
|> std.record.values
|> std.array.map (std_record_get "config")
|> std.record.merge_all
from nickel.
@uhlajs Yes indeed I somehow missed that proposal as I saw @yannham's snippet was still removing the inputs fields.
I think this is quite clean. This is what you would do in cuelang
for example.
from nickel.
Related Issues (20)
- Confusing merge behaviour in `std.record.{update,remove}` HOT 1
- `std.string.find_all` HOT 1
- Add `record.has_field_all` and `record.fields_all` HOT 1
- Add raw strings
- NLS crashes on recursive definitions HOT 11
- Proposal: freeze recursive records upon insert/remove/update/map
- NLS reports same diagnostic message multiple times HOT 3
- VSCode extension does not show hints for functions annotated with record contract HOT 1
- VSCode Extension should not disregard cursor position and collapse all symbol breadcrumbs to "..." in all occasions
- Weird `nix develop` error HOT 4
- Strange error in std.array.any HOT 2
- Provide contract interface to pyckel HOT 1
- Add functions to create and generically unwrap enum variants HOT 1
- Inconsistent behaviour while loading multiple files vs single file HOT 2
- Getting `record_insert: tried to extend but already exists` when using patterns with duplicate bound variables HOT 2
- Memory leak with long-running nickel process HOT 1
- Opaque foreign values
- Update documentation for new pattern features
- Accessing late-bind field fails HOT 6
- Context-sensitive completion in match cases
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from nickel.