cue-lang / cue

The home of the CUE language! Validate and define text-based and dynamic configuration
https://cuelang.org
Apache License 2.0
5.03k stars 287 forks source link

cue: improve documentation for Value.UnifyAccept #2300

Open kortschak opened 1 year ago

kortschak commented 1 year ago

What version of CUE are you using (cue version)?

Not using cue as an executable but rather via the Go API.

$ grep cuelang go.mod                        
    cuelang.org/go v0.6.0-0.dev

Does this issue reproduce with the latest stable release?

Yes

What did you do?

Using the Go API I would like to be able to unify a set of values while ignoring some fields that are only transiently generated but useful to retain for logging purposes.

For example if I have

{
    sum: "a31c34da"
    important1: "value1"
    config: {
        part1: "value2"
    }
} unify ignoring sum {
    sum: "ae20a82c"
    important2: "value3"
}

I would like to be able to resolve this to something like

{
    sum: "a31c34da"|"ae20a82c"
    config: {
        part1: "value2"
    }
    important1: "value1"
    important2: "value3"
}

or similar with the sum field omitted.

It looks to me from the documentation that something like this is supported by cue.Value.UnifyAccept, but how to use it to do this is unclear and I have not been able to get it to work in my experiments. Can the documentation be clarified for this?

What did you expect to see?

Clear explanation of how to use UnifyAccept.

What did you see instead?

Limited explanation. I was able to find one use of it in the tree and no tests, so it is difficult to know how to use it, or even whether my interpretation that it is usable for this is correct.

myitcv commented 1 year ago

I'm not altogether certain there isn't a bug lurking here given that method doesn't have tests.

myitcv commented 1 year ago

Oops, that sent early. Thanks for the report, @kortschak.

myitcv commented 1 year ago

Noting that https://github.com/cue-lang/cue/issues/454 is, I think, effectively the same as what you would like to achieve in this case.

That said, the UnifyAccept docs appear (per @mpvl) to be wrong. They should be improved, and a test added. Which we will aim to do in v0.6.0.

kortschak commented 1 year ago

Thanks. I'm not sure that #454 does exactly what I was looking for since it requires that fields be missing as opposed to the situation I have where I'd like explicitly not missing and disagreeing fields to be ignored, unless I'm missing something in that proposal for how to do that.

mvdan commented 1 year ago

@kortschak with a downcast, you can take a CUE struct and remove any fields which are not in another struct. Roger added an example in https://github.com/cue-lang/cue/issues/454#issuecomment-1313884145 which I find useful.

Going from the example you shared at the top, here are my thoughts on your scenario: https://tip.cuelang.org/play/?id=qFlvpZpJztc#cue@export@cue

If you want to simply omit the field sum, then the second approach works for you today, and I don't think downcasting would help that much. You can use a similar mechanism to omit a set of fields only, including pattern matching.

If you want to instead omit everything except a set of fields (what you could call an "allowlist" filter instead of a "blocklist"), then I think the third approach with downcasting would be better. It really depends on whether your data fields are a well-defined set or not.

mvdan commented 1 year ago

I'm going to remove this issue from the v0.6.0 milestone, since we want to get that out very soon, and we're focusing on siginficant regressions and bugs, such as panics or bad output. I'm more than happy to help unblock you regardless, of course :)

kortschak commented 1 year ago

Thanks @mvdan. I'm not blocked by this — I am working around it — but yes, the #MinusSum approach looks like what I want.

myitcv commented 1 year ago

@kortschak in your original description, you have two values (which I will refer to as a and b):

a: {
    sum:        "a31c34da"
    important1: "value1"
    config: {
        part1: "value2"
    }
}

b: {
    sum:        "ae20a82c"
    important2: "value3"
}

Do you know the schema of a and b? Or is everything apart from the sum field unknown? I'm assuming you do know something about the schemas because your expectation is that the unification of a modulo sum and b modulo sum should succeed.

Linking back to my thinking on how/why downcast might be related, therefore consider:

https://tip.cuelang.org/play/?id=U2kBHrwp3KP#cue@export@cue

If, however, you are looking for something more "automatic" where nothing is known about a and b, then it might be (per @mpvl) that what you are after is something closer to anti-unification (#7, #15, #2231).

Are you able to share a bit more context on the "inputs" a and b here? And also where/how the result will be used?

kortschak commented 1 year ago

@myitcv The types and the skeleton of the schema that I'm using are here https://go.dev/play/p/2nyCaScb9O7. There are a few other parts, but they are largely irrelevant to this issue. The Sum field in the Kernel, Module and Service structs is the field that can conflict. Conflicts in any other fields noted in the schema indicate an invalid configuration.

myitcv commented 1 year ago

The Sum field in the Kernel, Module and Service structs is the field that can conflict. Conflicts in any other fields noted in the schema indicate an invalid configuration.

Thanks for sharing this. To further help my understanding, what does it mean to unify a Kernel, Module and Service? They seem like quite distinct things, and I can't say I would have guessed that it could/should be possible to unify them.

kortschak commented 1 year ago

Oh, sorry, that is a crucial part that I missed out. The application takes configuration snippets (as TOML) and aggregates them in to a single configuration (a valid System), so the unifications are S₁, S₂, S₃ etc → S.

myitcv commented 1 year ago

Ah I see now. In which case I would suggest @mvdan's approach of #MinusSum is a reasonable one in the absence of downcast: you are dealing with concrete data (only regular fields) and the conflict is known to only occur at the top level.

The reason downcast will, in general, be more useful is that the "removal" of a field might need to occur at a nested point, in which case specifying "recursive" comprehensions gets messy very fast.

kortschak commented 1 year ago

Thanks. At the moment I'm just removing the field in Go before the validation and then replacing after. This is ugly, but simple. downcast looks similarly simple. (When) will it be added if ever?

myitcv commented 1 year ago

downcast looks similarly simple. (When) will it be added if ever?

I've added a comment to https://github.com/cue-lang/cue/issues/943 so that it is considered as part of that batch of builtins. We are planning to look through that list as part of the v0.7.0 changes, so will know more on timings/target release then.

sdboyer commented 1 year ago

Just want to register a general interest in builtins in this area. I realize there may be nuanced differences, but when i look at cut, downcast, #MinusSum between here and #454, they all strike me as being in a similar vein to TypeScript's Omit: given a definition of a type-thing, make a new type-thing that lacks certain fields/keys.

This could be quite valuable for reducing copypasta in some of our user flows.

myitcv commented 1 year ago

they all strike me as being in a similar vein to TypeScript's Omit:

With one important distinction to at least my understanding: downcast as proposed would be recursive in its implementation. As far as I can tell, Omit applies at the top level of Type (its first type parameter).

@sdboyer thanks for registering the interesting. I suggest we continue the conversation over in #943.