Skip to content

fix(engine): return the cue values merge error if non nil#331

Merged
stefanprodan merged 2 commits into
stefanprodan:mainfrom
GeorgeMac:gm/values-merge-report-error
Feb 5, 2024
Merged

fix(engine): return the cue values merge error if non nil#331
stefanprodan merged 2 commits into
stefanprodan:mainfrom
GeorgeMac:gm/values-merge-report-error

Conversation

@GeorgeMac

@GeorgeMac GeorgeMac commented Feb 1, 2024

Copy link
Copy Markdown
Contributor

I found a rather confusing error while making changes to some values.cue default definitions, which led me to investigate (happy to open an accompanying issue if that is useful):

10:13AM ERR build failed:
explicit error (_|_ literal) in source:
    ./values.cue:2:9

The change proposed returns the cue Values error if not nil, before attempting to write out the generated value.

The problem is in my values.cue file. It was that I had put a concrete list in my default values that wasn't compatible with my changes in the overriden values. For example:

// in default values.cue:
values: {
  list: []
}

// in overridden values.cue:
values: {
  list: [{ name: "some other entry" }]
}

I figured this had something to do with CUE merging my default and supplied values files together.
The reference ./values.cue:2:9 made no sense relative to my values files, so I assumed it was referring to some other intermediate generated file which led me to find the module builder. When I put in a debug line to print out the finalVal I found in had the contents (literall bottom value with a comment):

_|_ // values.list: incompatible list lengths (0 and 1)

Changing the builder to return the value error if not nil results in timoni failing like so:

10:14AM ERR values.list: incompatible list lengths (0 and 1)

@GeorgeMac GeorgeMac changed the title fix(engine): return values merge error if non nil fix(engine): return the cue values merge error if non nil Feb 1, 2024
@stefanprodan stefanprodan added the area/engine CUE engine related issues and pull requests label Feb 5, 2024
@stefanprodan

Copy link
Copy Markdown
Owner

@GeorgeMac thank you for this. Just as a remark that the values.cue from the module root should be empty. The best place to provide defaults, is in the #Config definition. An example for this is in the https://github.com/stefanprodan/timoni/tree/main/blueprints/starter.

@stefanprodan stefanprodan left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Thanks @GeorgeMac 🏅

@stefanprodan stefanprodan merged commit df14aa4 into stefanprodan:main Feb 5, 2024
@GeorgeMac GeorgeMac deleted the gm/values-merge-report-error branch February 5, 2024 16:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/engine CUE engine related issues and pull requests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants