fix: seperated _update_cluster_fn from _process_operation_fn to displ…

[SteBaum]

fix: seperated _update_cluster_fn from _process_operation_fn to display deployment operations

Which issue(s) this PR fixes

Fixes None

Additional comments

Before this PR the tdp deploy or tdp deploy --mock-deploy does not display the -install operations although they are executed and written in the database since they do not provide cluster_status_logs directly. Therefore in order to show these commands too, in the iterator the _process_operation_fn must be separated in two. one function which executes the operation and a second one which updates the status_logs.

Agreements

• I hereby declare this contribution to be licensed under the Apache License Version 2.0, January 2004.
• I hereby agree to grant TOSIT a copyright license to use my contributions.

[PaulFarault]

If I understand correctly, you're saying that the CLI doesn't display any message in the console for the *_install operations because their _process_operation_fn aren't returning any logs to append to the sch_status_log table.

If so, extracting the log generation logic from the _process_operation_fn is a good refactor. However, the DeploymentIterator signature likely shouldn't change. Why return two functions instead of simply doing the following:

for operation_rec, process_operation_fn in deployment_iterator:
    dao.session.commit()  # Update deployment and current operation status to RUNNING and next operations to PENDING
-   if process_operation_fn and (cluster_status_logs := process_operation_fn()):
+   if process_operation_fn:
        click.echo(
            f"Operation {operation_rec.operation} is {operation_rec.state} {'for hosts: ' + operation_rec.host if operation_rec.host is not None else ''}"
        )
+       if cluster_status_logs := process_operation_fn():
-       dao.session.add_all(cluster_status_logs)
+           dao.session.add_all(cluster_status_logs)
    dao.session.commit()  # Update operation status to SUCCESS, FAILURE or HELD
    

            dao.session.add_all(cluster_status_logs)
    dao.session.commit()  # Update operation status to SUCCESS, FAILURE or HELD

[SteBaum]

If I understand correctly, you're saying that the CLI doesn't display any message in the console for the *_install operations because their _process_operation_fn aren't returning any logs to append to the sch_status_log table.

If so, extracting the log generation logic from the _process_operation_fn is a good refactor. However, the DeploymentIterator signature likely shouldn't change. Why return two functions instead of simply doing the following:

for operation_rec, process_operation_fn in deployment_iterator:
    dao.session.commit()  # Update deployment and current operation status to RUNNING and next operations to PENDING
-   if process_operation_fn and (cluster_status_logs := process_operation_fn()):
+   if process_operation_fn:
        click.echo(
            f"Operation {operation_rec.operation} is {operation_rec.state} {'for hosts: ' + operation_rec.host if operation_rec.host is not None else ''}"
        )
+       if cluster_status_logs := process_operation_fn():
-       dao.session.add_all(cluster_status_logs)
+           dao.session.add_all(cluster_status_logs)
    dao.session.commit()  # Update operation status to SUCCESS, FAILURE or HELD
    

            dao.session.add_all(cluster_status_logs)
    dao.session.commit()  # Update operation status to SUCCESS, FAILURE or HELD

The seperation makes sens since executing the operations and creating the status logs are two different operations. However I have modified as you said to keep it as close as what was before and if the separation takes place it will be n another PR.