Improve the description/help text... by amolpati30 · Pull Request #786 · theforeman/foremanctl

amolpati30 · 2026-08-26T09:27:45Z

Improve the help text so that --help provides clear and accurate descriptions for the following parameters:

--log-level
--foreman-log-level
--foreman-proxy-log-level
--valkey-log-level
--pulp-log-level

Previously, the descriptions were not clear. They are now more precise and provide better information about the purpose and behavior of each parameter.

lzap · 2026-08-26T11:24:33Z

    help: Name of the PAM service to use for IPA authentication
  foreman_log_level:
-    help: Log level for Foreman. Defaults to the global log level.
+    help: Default log level for all services. unless overridden by the service-specific parameters.


Suggested change

help: Default log level for all services. unless overridden by the service-specific parameters.

help: Default log level for all services, unless overridden by the service-specific parameters.

This line is not applicable to Foreman, so I added a different line that is applicable to Foreman.

shubhamsg199 · 2026-08-26T12:56:41Z

      - fatal
  foreman_proxy_log_level:
-    help: Log level for Foreman Proxy. Defaults to the global log level.
+    help: Defaults to the global log level, can be set independently. Once set, it overrides --log-level and is unaffected by later changes.


Suggested change

help: Defaults to the global log level, can be set independently. Once set, it overrides --log-level and is unaffected by later changes.

help: Log level for Foreman Proxy. Defaults to the global log level, can be set independently. Once set, it overrides --log-level and is unaffected by later changes.

I intentionally removed that part because the parameter is already foreman_proxy, so I don't think it's necessary to mention that the log is for foreman_proxy.

I tend to disagree with your reasoning. Yes, the parameter name makes it clear, but we should still describe the functionality.

cc @jafiala & @ekohl for additional opinions.

My first step was to try myself to see how it looks and I'm surprised that I can't even get the parameter to show up:

$ git describe 3.1.0-develop-29-g978f0a1db5b7 $ ./foremanctl deploy --help usage: foremanctl deploy [-h] [-v] [--external-authentication {ipa,ipa_with_api}] [--external-authentication-pam-service EXTERNAL_AUTHENTICATION_PAM_SERVICE] [--add-feature FEATURES] [--flavor {katello}] [--foreman-log-level {debug,info,warn,error,fatal}] [--content-export-path PULP_EXPORT_PATHS] [--content-import-path PULP_IMPORT_PATHS] [--remove-feature FEATURES] Install Server options: -h, --help show this help message and exit -v, --verbose verbose output --external-authentication {ipa,ipa_with_api} External authentication method to use (persisted) --external-authentication-pam-service EXTERNAL_AUTHENTICATION_PAM_SERVICE Name of the PAM service to use for IPA authentication (persisted) --add-feature FEATURES Additional features to enable in this deployment. (persisted) --flavor {katello} Base flavor to use in this deployment. (persisted) --foreman-log-level {debug,info,warn,error,fatal} Log level for Foreman. Defaults to the global log level. (persisted) --content-export-path PULP_EXPORT_PATHS Extra file path that Pulp can use for content exports. Argument may be used more than once. (persisted) --content-import-path PULP_IMPORT_PATHS Extra file path that Pulp can use for content imports. Argument may be used more than once. (persisted) --remove-feature FEATURES Additional features to disable in this deployment. (persisted) Parameters marked as (persisted) can be reset by --reset-<parameter-name>.

I know I don't have the Foreman Proxy feature enabled, but this hurts discoverability.

As a user, how am I supposed to know this parameter exists?

Now on the more concrete question: I do think the description is nice. They can be on different lines and being explicit helps IMHO. We have pretty much the same thing for --external-authentication and I wouldn't know what else to write there. 👍 for keeping the description part.

@stejskalleos
If you insist, I can add a line describing the functionality of the parameter.

shubhamsg199 · 2026-08-26T12:56:53Z

      - fatal
  valkey_log_level:
-    help: Log level for Valkey.
+    help: Defaults to the global log level, can be set independently. Once set, it overrides --log-level and is unaffected by later changes.


Suggested change

help: Defaults to the global log level, can be set independently. Once set, it overrides --log-level and is unaffected by later changes.

help: Log level for Valkey. Defaults to the global log level, can be set independently. Once set, it overrides --log-level and is unaffected by later changes.

I intentionally removed that part because the parameter is already Valkey, so I don't think it's necessary to mention that the log is for Valkey.

shubhamsg199 · 2026-08-26T12:57:07Z

      - nothing
  pulp_log_level:
-    help: Log level for Pulp. Defaults to the global log level.
+    help: Defaults to the global log level, can be set independently. Once set, it overrides --log-level and is unaffected by later changes.


Suggested change

help: Defaults to the global log level, can be set independently. Once set, it overrides --log-level and is unaffected by later changes.

help: Log level for Pulp. Defaults to the global log level, can be set independently. Once set, it overrides --log-level and is unaffected by later changes.

Same here; it is not required, as mentioned above.

shubhamsg199 · 2026-08-26T12:57:30Z

    help: Name of the PAM service to use for IPA authentication
  foreman_log_level:
-    help: Log level for Foreman. Defaults to the global log level.
+    help: Default log level for all services. unless overridden by the service-specific parameters.


Suggested change

help: Default log level for all services. unless overridden by the service-specific parameters.

help: Log level for Foreman. Default log level for all services. unless overridden by the service-specific parameters.

Same here; it is not required, as mentioned above.

--log-level --foreman-log-level --foreman-proxy-log-level --valkey-log-level --pulp-log-level

stejskalleos · 2026-08-27T06:36:02Z

 | Parameter | Description |
 | ----------| ----------- |
-| `--log-level` | Default log level for all services. Accepted values: `debug`, `info`, `warn`, `error`, `fatal`. Defaults to `info`. |
+| `--log-level` | Default log level for all services, unless overridden by the service-specific parameters. Accepted values: `debug`, `info`, `warn`, `error`, `fatal`. Defaults to `info`. | 


Suggested change

| `--log-level` | Default log level for all services, unless overridden by the service-specific parameters. Accepted values: `debug`, `info`, `warn`, `error`, `fatal`. Defaults to `info`. |

| `--log-level` | Default log level for all services, unless overridden by the service-specific parameters. Accepted values: `debug`, `info`, `warn`, `error`, `fatal`. Defaults to `info`. |

stejskalleos · 2026-08-27T06:39:29Z

      - fatal
  foreman_proxy_log_level:
-    help: Log level for Foreman Proxy. Defaults to the global log level.
+    help: Defaults to the global log level, can be set independently. Once set, it overrides --log-level and is unaffected by later changes.


I tend to disagree with your reasoning. Yes, the parameter name makes it clear, but we should still describe the functionality.

cc @jafiala & @ekohl for additional opinions.

pr-processor Bot added the Not yet reviewed label Aug 26, 2026

lzap suggested changes Aug 26, 2026

View reviewed changes

pr-processor Bot added Waiting on contributor and removed Not yet reviewed labels Aug 26, 2026

stejskalleos requested changes Aug 26, 2026

View reviewed changes

Comment thread docs/user/parameters.md Outdated

Comment thread src/playbooks/_logging/metadata.obsah.yaml Outdated

shubhamsg199 reviewed Aug 26, 2026

View reviewed changes

Comment thread docs/user/parameters.md Outdated

shubhamsg199 reviewed Aug 26, 2026

View reviewed changes

Improve the description/help text for the following parameters:

19ca6c4

--log-level --foreman-log-level --foreman-proxy-log-level --valkey-log-level --pulp-log-level

amolpati30 force-pushed the improve_text_for_log_level_parameter branch from 216ba94 to 19ca6c4 Compare August 26, 2026 19:16

pr-processor Bot added Needs re-review and removed Waiting on contributor Needs re-review labels Aug 26, 2026

stejskalleos requested changes Aug 27, 2026

View reviewed changes

pr-processor Bot added the Waiting on contributor label Aug 27, 2026

	help: Default log level for all services. unless overridden by the service-specific parameters.
	help: Default log level for all services, unless overridden by the service-specific parameters.

	help: Defaults to the global log level, can be set independently. Once set, it overrides --log-level and is unaffected by later changes.
	help: Log level for Foreman Proxy. Defaults to the global log level, can be set independently. Once set, it overrides --log-level and is unaffected by later changes.

	help: Defaults to the global log level, can be set independently. Once set, it overrides --log-level and is unaffected by later changes.
	help: Log level for Valkey. Defaults to the global log level, can be set independently. Once set, it overrides --log-level and is unaffected by later changes.

	help: Defaults to the global log level, can be set independently. Once set, it overrides --log-level and is unaffected by later changes.
	help: Log level for Pulp. Defaults to the global log level, can be set independently. Once set, it overrides --log-level and is unaffected by later changes.

	help: Default log level for all services. unless overridden by the service-specific parameters.
	help: Log level for Foreman. Default log level for all services. unless overridden by the service-specific parameters.

	\| `--log-level` \| Default log level for all services, unless overridden by the service-specific parameters. Accepted values: `debug`, `info`, `warn`, `error`, `fatal`. Defaults to `info`. \|
	\| `--log-level` \| Default log level for all services, unless overridden by the service-specific parameters. Accepted values: `debug`, `info`, `warn`, `error`, `fatal`. Defaults to `info`. \|

Uh oh!

Conversation

amolpati30 commented Aug 26, 2026

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants