Skip to content

Increase the fallback for microSALT version command#4974

Open
RasmusBurge-CG wants to merge 6 commits intomasterfrom
improve-error-handling-in-mircosalt
Open

Increase the fallback for microSALT version command#4974
RasmusBurge-CG wants to merge 6 commits intomasterfrom
improve-error-handling-in-mircosalt

Conversation

@RasmusBurge-CG
Copy link
Copy Markdown
Contributor

@RasmusBurge-CG RasmusBurge-CG commented Mar 24, 2026

Description

The general Exception could not handle the scenario when the exit code of get_workflow_version was unsuccessful.

Changed

  • We now except that the get version command can fail for microSALT, and we persist the fallback version 0.0.0.

How to prepare for test

  • Ssh to relevant server (depending on type of change)
  • Use stage: us
  • Paxa the environment: paxa
  • Install on stage (example for Hasta):
    bash /home/proj/production/servers/resources/hasta.scilifelab.se/update-tool-stage.sh -e S_cg -t cg -b [THIS-BRANCH-NAME] -a

How to test

  • Do ...

Expected test outcome

  • Check that ...
  • Take a screenshot and attach or copy/paste the output.

Review

  • Tests executed by
  • "Merge and deploy" approved by
    Thanks for filling in who performed the code review and the test!

This version is a

  • MAJOR - when you make incompatible API changes
  • MINOR - when you add functionality in a backwards compatible manner
  • PATCH - when you make backwards compatible bug fixes or documentation/instructions

Implementation Plan

  • Document in ...
  • Deploy this branch on ...
  • Inform to ...

@RasmusBurge-CG RasmusBurge-CG requested a review from a team as a code owner March 24, 2026 10:14
@RasmusBurge-CG
Copy link
Copy Markdown
Contributor Author

CodeFactor is warning about shell=True. As this is used in other tests and was accepted before, this warning can be disregarded.

Copy link
Copy Markdown
Contributor

@diitaz93 diitaz93 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this is the intended behaviour, it looks good 👍

@RasmusBurge-CG RasmusBurge-CG requested a review from diitaz93 March 25, 2026 14:38
@RasmusBurge-CG
Copy link
Copy Markdown
Contributor Author

Hi @diitaz93, I sent this back for review after some medium-sized refactoring. Curious to hear what you think about the update. :)

@sonarqubecloud
Copy link
Copy Markdown

Comment on lines +49 to +58

except Exception as e:

stderr = ""
if isinstance(e, subprocess.CalledProcessError):
stderr = e.stderr.decode("utf-8").rstrip()

LOG.warning(
f"Could not retrieve {case_config.workflow} workflow version: {e} : {stderr}"
)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we remove the spaces? not a fan of sparse code

Suggested change
except Exception as e:
stderr = ""
if isinstance(e, subprocess.CalledProcessError):
stderr = e.stderr.decode("utf-8").rstrip()
LOG.warning(
f"Could not retrieve {case_config.workflow} workflow version: {e} : {stderr}"
)
except Exception as e:
stderr = ""
if isinstance(e, subprocess.CalledProcessError):
stderr = e.stderr.decode("utf-8").rstrip()
LOG.warning(
f"Could not retrieve {case_config.workflow} workflow version: {e} : {stderr}"
)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants