Possible improvements

[EGI-ILM]

This is just a suggestion, to register some possible improvements for the code that I have seen while I worked on #127 :

• Review code to eliminate additional possible code duplication.
• It could be beneficial to provide some progress feedback to the user (e.g. during the execution of time consuming commands such as fedcloud endpoint projects -a). even if it is just a printed symbol that changes or a printed dot "." at the time a new site is accessed. This should not interfere with the standard output or could be a log level option.
• More solid exception management, as referred in Better exception catching #128 . Generic exceptions are captured in several parts so it makes difficult to treat them separately in an adequate manner.
• Error messages should be unified. The work I did on PR Issue #127 #129 in this respect should be considered temporary.
• It would be beneficial to use a proper logging framework, and fix the lack of logs.
• Some parts of the code could benefit from concurrency. I am aware that Python does not excel at multithreading processing but, for example, the performance of fedcloud endpoint projects -a can greatly improve if all sites are contacted asynchronously.

[tdviet]

This is just a suggestion, to register some possible improvements for the code that I have seen while I worked on #127 :

Thanks for useful suggestions. Some discussions are inline:

• Review code to eliminate additional possible code duplication.

Fully agree.

• It could be beneficial to provide some progress feedback to the user (e.g. during the execution of time consuming commands such as fedcloud endpoint projects -a). even if it is just a printed symbol that changes or a printed dot "." at the time a new site is accessed. This should not interfere with the standard output or could be a log level option.

Agree. In normal condition, most of commands should respond in acceptable time, so this feature is not critical. fedcloud endpoint projects -a is an exception, but we can improve it by concurrency, see below. However, if there are problems with some sites (especially Connection Timeout it takes ~ 30s by default before returning errors), the respond time is longer than expected.

• More solid exception management, as referred in Better exception catching #128 . Generic exceptions are captured in several parts so it makes difficult to treat them separately in an adequate manner.

Agree, too.

• Error messages should be unified. The work I did on PR Issue #127 #129 in this respect should be considered temporary.
• It would be beneficial to use a proper logging framework, and fix the lack of logs.

Agree, too.

• Some parts of the code could benefit from concurrency. I am aware that Python does not excel at multithreading processing but, for example, the performance of fedcloud endpoint projects -a can greatly improve if all sites are contacted asynchronously.

Concurrency should not be problem for Python, it has been implemented, for example for fedcloud openstack -a command. https://github.com/tdviet/fedcloudclient/blob/master/fedcloudclient/openstack.py#L260-303

[EGI-ILM]

Great!

Also, I saw time ago that fedcloud token check does not seem to work as I expected on OIDC. So I ended up using:

printf '%(%F %T)T\n' `oidc-token --env $OIDC_AGENT_ACCOUNT | grep -oP '(?<=OIDC_EXP=).*?(?=;)'`

instead

[tdviet]

I expect the access token from oidc-agent is always valid, as the oidc-agent will check and update automatically when expires, so the fedcloud token check only check the token provided via --oidc-access-token

[EGI-ILM]

Oh, I see. The error message was a bit misleading, as it mentioned that OIDC access token is required, and I have one:

i@tckr:~$ fedcloud token check
OIDC access token or refresh token required

Although I might not be getting the correct answer (or I am calling it incorrectly):

i@tckr:~$ fedcloud token check --oidc-access-token egi
Error: Invalid access token.

[tdviet]

The option --oidc-access-token should be accompanied with the token in free text, not oidc-agent account name. Try

fedcloud token check --oidc-access-token `oidc-token egi`

[EGI-ILM]

Thanks, so I called it wrongly then.

If you find it reasonable/useful, perhaps the error message of token check could point to this, something like:

A refresh token is not available.
If you are using OIDC, use parameter "--oidc-access-token" e.g. fedcloud token check --oidc-access-token `oidc-token egi`

[tdviet]

It is an awkward question, can I ask who are you from EGI? I try to decode ILM but it does not match any name nor position from https://www.egi.eu/about/egi-foundation/team/.

[EGI-ILM]

:-D Sorry, you are right that I did not introduce myself and we are exchanging quite a few comments. I will send you an email.

[EGI-ILM]

Following this list of suggestions, I would like to add some other ideas:

• Implement a "-f --format" argument to output the results in a specific format (e.g. the default table, JSON, YAML, CSV, grep-able text, Terraform, etc) . This can be very useful for users. For example, Fedcloudclient can absorb the functionality I have created in my Terraform script so it can provide out-of-the-box Terraform files to manage infrastructure.
• Unified management of IGTF certificates. I am following the recommendations for production use and I install the certificates via a script. This has some problems: (i) It is only valid for apt-based systems. (ii) It appends the certificates in the keystore every time it is executed. (iii) In my case, they are installed in my general keystore, so it makes the system trust IGTF certificates for tasks unrelated to Fedcloud. Perhaps it would be better to "cache" the certificates in an alternative keystore and set this keystore during Fedcloud execution. This way, it can be integrated within Fedcloudclient with no additional installation, be system agnostic and avoid messing with the default system keystore. I guess it can be a simplified version of what is already done in another script , but transparently integrated within the program.

[tdviet]

This issue mixed different improvements so new issues created for clarification. Discussion should be done in the new issues.

• Improving certificate management #143
• Pluggable output formatters #140