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

[Question] Convention to follow to log or report error #450

Open
cmoulliard opened this issue Nov 15, 2024 · 0 comments
Open

[Question] Convention to follow to log or report error #450

cmoulliard opened this issue Nov 15, 2024 · 0 comments
Labels
question Further information is requested

Comments

@cmoulliard
Copy link
Contributor

cmoulliard commented Nov 15, 2024

Questions

Which convention should we adopt/follow to log error when we call a CLI command.

@nabuskey @greghaynes @blakeromano @punkwalker

Return error to the caller (yes/no)

Do we have to format the errors within the function called and return the error to the caller ?

fmt.Errorf("building labels with key %s and value %s : %w", v1alpha1.CLISecretLabelKey, v1alpha1.CLISecretLabelValue, err)

Log messages or errors within a function called (yes/no)

Can we also log messages (info, debug, warning or error) within a function called by a command ?.

Log verbose level

I'm a bit confused about what logr reports when we set the verbosity to V(1)

logger.V(1).Info(fmt.Sprintf("Got the context for the cluster: %s.", cluster))

This generates =>

Nov 15 13:32:07 DEBUG+3 Got the context for the cluster: my-konflux. 

How such a V level int is mapped to the following values: debug, info, warn, and error listed by a command using -h

❯ ./idpbuilder get clusters -h
Get idp clusters

Usage:
  idpbuilder get clusters [flags]

Flags:
  -h, --help   help for clusters

Global Flags:
      --color              Enable colored log messages.
      --kubePath string    kube config file Path.
  -l, --log-level string   Set the log verbosity. Supported values are: debug, info, warn, and error. (default "info")

Os exit (yes/no)

Should we also call os.exit when a blocking error is discovered (example : it is needed to have access to a kube cluster but we cannot) within a function called by a CLI command

See example hereafter

var ClustersCmd = &cobra.Command{
	Use:     "clusters",
	Short:   "Get idp clusters",
	Long:    ``,
	RunE:    list,
	PreRunE: preClustersE,
}

func preClustersE(cmd *cobra.Command, args []string) error {
	return helpers.SetLogger()
}

func list(cmd *cobra.Command, args []string) error {
	clusters, err := populateClusterList()
	if err != nil {
		return err
	} else {
		// Convert the list of the clusters to Table of clusters
		printTable(printers.PrintOptions{}, generateClusterTable(clusters))
		return nil
	}
}

func populateClusterList() ([]Cluster, error) {
	logger := helpers.CmdLogger

	detectOpt, err := util.DetectKindNodeProvider()
	if err != nil {
		logger.Error(err, "failed to detect the provider.")
		os.Exit(1)
	}

@nabuskey @nimakaviani @greghaynes

@cmoulliard cmoulliard added the question Further information is requested label Nov 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

1 participant