mondoohq / cnquery

open source, cloud-native, graph-based asset inventory
https://cnquery.io
Other
293 stars 20 forks source link

Indicate that values are not set #1943

Open arlimus opened 11 months ago

arlimus commented 11 months ago

When using fields in resources that have not been set, we currently print confusing indications for users:

cnquery> audit.advisory{*}
audit.advisory: {
  description: cannot convert primitive with NO type information
  worstScore: cannot convert primitive with NO type information
  published: cannot convert primitive with NO type information
  mrn: cannot convert primitive with NO type information
  id: cannot convert primitive with NO type information
  title: cannot convert primitive with NO type information
  modified: cannot convert primitive with NO type information
}

What the system is trying to tell us: This resource has none of its (mandatory) values set. In this specific example, we have a resource that should be entirely pre-initialized, but has no values at all. There are also mixed use-cases, where partial values may be provided or where fields depend on values that have not been set.

None of these cases are currently handled in v9.

There are 2 possible solutions:

A: Indicate missing fields

Soft indications that no data is provided without errors or failure

// FYI: I'm comments to indicate the gray color users see!
// We are not printing comments, just gray text to soft-fail this
cnquery> audit.advisory{*}
audit.advisory: {
  description: // not set
  worstScore: // not set
  published: // not set
  mrn: // not set
  id: // not set
  title: // not set
  modified: // not set
}

B: Error on missing fields

Same as above, but every field counts as an error

# I'm only using the highlighter to indicate errors
cnquery> audit.advisory{*}
audit.advisory: {
  description: not set
  worstScore: not set
  published: not set
  mrn: not set
  id: not set
  title: not set
  modified: not set
}

C: Error on resource creation

This is what v8 does, just with better error messages for v9:

cnquery> audit.advisory{*}
Query encountered errors:
failed to create resource 'audit.advisory': no values provided for mandatory fields 'mrn', 'id', 'title', and 3 others
audit.advisory: no data available

Solution

We will need to decide on which path to take for v9.

Not a v9 release blocker, as it doesn't break behavior.

Supersedes https://github.com/mondoohq/cnquery/issues/1721

Closes https://github.com/mondoohq/cnquery/pull/1894 in favor of a solution that addresses this holistically.

chris-rock commented 11 months ago

I very much likes A. I think the field should have a specific error or type that is rendered custom. In this case "not set". I like the grayed out printing. We have the same case with permissions e.g. user has no permission to retrieve the data. There is nothing the user can do, so we should not print this in a full blown error but instead make it a helpful message: hey you do not have permissions to see this part of the graph.