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

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

Merged

Conversation

GeorgeMac
Copy link
Sponsor Contributor

@GeorgeMac GeorgeMac commented Feb 1, 2024

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
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.

Copy link
Owner

@stefanprodan stefanprodan left a comment

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
4 checks passed
@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.

None yet

2 participants