Removed unusable log options and add new section to user_config_template

[LeiGlobus]

Description

Removed several log related options that do not work, and added a new log section where one can specify the user log path.

This new log path is passed as an environment variable to the UEP called GLOBUS_COMPUTE_LOG_PATH though it's debatable whether we want to use direct parameters fed via stdin instead which then in turn sets the env var in the UEP.

Need to move a few fixtures around as they are shared by new test.

Also wrapped cli.py so Exceptions are printed instead of being swallowed by Click and merely printing the --help message.

(TBD adding another test will remove this sentence after addition)

[sc-39546]

Type of change

• New feature (non-breaking change that adds functionality)
• Documentation update
• Code maintenance/cleanup

[chris-janidlo]

Haven't tested this yet, but looks good overall. Mostly small comments except for maybe some missing tests

[chris-janidlo]

Are there any tests verifying this new endpoint_manager behavior?

[LeiGlobus]

No tests for this yet, but the logic is fairly simple, and any errors will be fairly obvious (e.g. the UEP will ignore the config altogether). I'll look into whether it can be added to an existing test.

[chris-janidlo]

Logic/errors aside, if these env vars are a contractual invariant we want the CEP to always set---and we do, because of the fact that UEPs always expect them---we should have a test that verifies it actually happens.

[khk-globus]

• It is a subtle detail here to rely on the environment variable for the next function call. It's not wrong, just unexpected in the usual course, especially in context of the other branch, which sets its environment variable at the end of the block. A simple comment (inline, if feasible) might help.
• I also observe that GLOBUS_COMPUTE_LOG_PATH is used in variable form within ensure_log_path. For single-point-of-authority, this should do similar.
• You may or may not like a stylistic choice to unindent a block, but I offer it as an option:

  if gc_dir_val := _conf.get("paths", {}).get("gc_dir"):
      ...
  ...
  if log_path_val := _conf.get("paths", {}).get("endpoint_log"):
      ...

[khk-globus]

In the common case where endpoint_log is not specified, this represents a slight Shlemiel approach; consider ensure_log_path() to after ensure_compute_dir() has been invoked.

[LeiGlobus]

Rewrote the order in this section to make it flow better with the dependency necessary

[khk-globus]

• In the first branch, log_path is basically just the pathlib-ization of log_dir. That's unexpected, then, in line 289 that requires that log_path be not a directory.
• A simplification of the if-conditional might be:

  if log_dir := os.environ.get(LOG_PATH_ENV, "").strip():

• I find it unexpected that one branch relies on an environment variable while the other does not. Consider making this function take no arguments and only rely on environment variables. (Especially since we're nominally always setting GLOBUS_COMPUTE_USER_DIR now, yeah?)

[LeiGlobus]

Needs a bit more discussion offline.

[LeiGlobus]

Rewritten

[khk-globus]

Given recent iterations, GLOBUS_COMPUTE_USER_DIR is no longer modifiable from the configuration. Thus, this section could be simplified with an else: branch. Consider:

if log_path_val := _conf.get("paths", {}).get("endpoint_log"):
    # Overrides the default in ensure_log_path()
    env[LOG_PATH_ENV] = log_path_val

if ep_dir_val := _conf.get("paths", {}).get("endpoint_dir"):
    log.info(f"Setting endpoint directory to {ep_dir_val}")
    ep_dir = pathlib.Path(ep_dir_val)
else:
    ep_dir = ensure_compute_dir() / ep_name

ep_dir.mkdir(mode=0o700, parents=True, exist_ok=True)
env[COMPUTE_EP_DIR_ENV] = str(ep_dir.resolve())

ep_log = ensure_log_path()

[khk-globus]

Given that ensure_log_path could change the value given to it by the environment variable (c.f., expandvars() and expanduser()), we should similarly update the environment variable.

[LeiGlobus]

Updated, but needs some discussion as to where the updating should be done.

[khk-globus]

Please address the incongruence of log_dir.

[khk-globus]

(Same comment as made in previous review iteration.)

[LeiGlobus]

Rewrote and simplified the logic