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

Provide more detailed error report about missing configuration value if it is computed #24444

Open
ov7a opened this issue Mar 22, 2023 · 11 comments
Labels
a:feature A new functionality in:configuration-model lazy api, domain object container in:provider-api property lazy provider MapProperty ListProperty DirectoryProperty re:comprehensibility reasonable errors and warnings, clear dsl, mental overload

Comments

@ov7a
Copy link
Member

ov7a commented Mar 22, 2023

Current Behavior

User gets an error about missing value. The information they get isn't complete: there is no indication that the value is computed. The solutions are misleading: user can't set computed value.

FAILURE: Build failed with an exception.

* What went wrong:
A problem was found with the configuration of task ':customJar' (type 'Jar').
  - Type 'org.gradle.api.tasks.bundling.Jar' property 'archiveFile' doesn't have a configured value.
    
    Reason: This property isn't marked as optional and no value has been configured.
    
    Possible solutions:
      1. Assign a value to 'archiveFile'.
      2. Mark property 'archiveFile' as optional.

Expected Behavior

User gets a descriptive error with a note about missing computed value and details about how it is computed and/or which configure values it depends on. Something like this:

A problem was found with the configuration of task ':customJar' (type 'Jar').
  - Type 'org.gradle.api.tasks.bundling.Jar' property 'archiveFile' doesn't have a configured value.
    
    Reason: This property isn't marked as optional and no value has been configured. This is a computed value based on 'destinationDirectory' and 'baseName'.
    
    Possible solutions:
      1. Assign a values to 'destinationDirectory' and 'baseName'.
      2. Mark property 'archiveFile' as optional.

Context

Current behavior might be confusing for new users. They can try to set a computed value and would get another error. See an example here.

Reproducer

@ov7a ov7a added a:feature A new functionality to-triage in:configuration-model lazy api, domain object container in:problems problems api in:provider-api property lazy provider MapProperty ListProperty DirectoryProperty pending:reproducer Indicates that the issue requires a reproducer or will be closed after 7 days and removed to-triage labels Mar 22, 2023
@ov7a ov7a removed the pending:reproducer Indicates that the issue requires a reproducer or will be closed after 7 days label Mar 29, 2023
@lptr lptr added the p:lazy-migration Issues covered by migration to an all-lazy API label May 30, 2023
@mlopatkin
Copy link
Member

@gradle/bt-execution task input validation is currently in your domain, do you plan to handle this issue? Configuration team is ready to help with the plumbing if any is missing.

@asodja
Copy link
Member

asodja commented Jun 27, 2023

I will add it to our list and we can triage it when Lóránt is back.

@lptr
Copy link
Member

lptr commented Jul 10, 2023

The change required here is mostly pending on the provenance tracking feature of the provider API. Once that feature is available, we can update the message here to better attribute the problem.

@lptr
Copy link
Member

lptr commented Jul 10, 2023

We should clarify that archiveFile is not a Property and cannot be set. We should clarify that already.

@lptr
Copy link
Member

lptr commented Jul 11, 2023

Just for clarification: I think what is meant here is that it's a property of the task instance in the JavaBean sense, which is true for archiveFile. The message overall is still misleading, though.

@lptr
Copy link
Member

lptr commented Jul 11, 2023

We should distinguish between null values and absent Providers. When we find one latter, we can check if the provider implements HasConfigurableValue. If so, we can add the suggestion to specify a value for it, otherwise not. That would remove the misleading suggestion to specify a value for archiveFile.

If we wanted to actually explain where the current provided value comes from, for that we need support for provenance tracking first, which is a bigger undertaking that we should design more holistically.

@lptr
Copy link
Member

lptr commented Jul 11, 2023

BTW, is "computed" the terminology we want to use when describing providers being fed other providers as inputs?

@big-guy
Copy link
Member

big-guy commented Jul 12, 2023

I don't think foo.set(otherProp) is "computing" anything, if that's what you mean. I would describe this as "otherProp provides the value of foo".

I could see saying "computed" when we fail to determine a value for a property. We use this in the documentation already to refer to "expensive computations" or "computed" values with zip. "determined" could also work in those contexts.

We could say "foo's value could not be computed from otherProp", but I would then expect we would say something like "otherProp's value could not be computed because its not known"

@FrauBoes
Copy link
Contributor

A partial fix (improving the misleading proposed solution) will be addressed by:

@lptr lptr removed p:lazy-migration Issues covered by migration to an all-lazy API @execution labels Jul 24, 2023
@lptr
Copy link
Member

lptr commented Jul 24, 2023

The rest is not a requirement for provider API migration, moving back to @gradle/bt-configuration-cache.

@bamboo bamboo added the 👋 team-triage Issues that need to be triaged by a specific team label Jul 26, 2023
@mlopatkin mlopatkin removed the 👋 team-triage Issues that need to be triaged by a specific team label Aug 1, 2023
@ov7a
Copy link
Member Author

ov7a commented Aug 4, 2023

@ov7a ov7a removed the in:problems problems api label Oct 5, 2023
@ov7a ov7a added the re:comprehensibility reasonable errors and warnings, clear dsl, mental overload label Oct 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a:feature A new functionality in:configuration-model lazy api, domain object container in:provider-api property lazy provider MapProperty ListProperty DirectoryProperty re:comprehensibility reasonable errors and warnings, clear dsl, mental overload
Projects
None yet
Development

No branches or pull requests

7 participants