Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feat: implements nextflow_version new parameter for job submission #171

Merged
merged 11 commits into from
Nov 28, 2024

Conversation

dapineyro
Copy link
Collaborator

@dapineyro dapineyro commented Nov 27, 2024

Overview

This PR implements the following new features:

  • New --nextflow-version parameter to select the Nextflow version to execute on CloudOS
  • Now it accepts cloudOS URLs with or without a trailing '/' (previous behaviour was failure when using URL with a trailing '/')

Jira

https://lifebit.atlassian.net/browse/LIF-1124

Acceptance criteria

See: https://lifebit.atlassian.net/wiki/spaces/DEL/pages/1223491604/LIF-1222+CloudOS+CLI+support+for+multiple+nextflow+versions+LP-16142#Proposed-state

Changes

  • Adds the new parameter --nextflow-version :
    • It allows for only three possible string values: '22.10.8' and '24.04.4'. (current supported versions on CloudOS) and 'latest' that selects '24.04.4' too. Default='22.10.8'
    • There is a warning (same as in the platform) when selecting other than '22.10.8'.
    • System tools, target-id, etl pipelines (modules) only allow for '22.10.8' (with message if using different)
  • Some messages have been improved for consistency
  • Some docstrings have been improved for consistency
  • UX improvement: the user can now specify the url with or without a trailing '/', so that now https://cloudos.lifebit.ai/ and https://cloudos.lifebit.ai are acceptable (previously, https://cloudos.lifebit.ai/ raised an error with no clear message)

Tests

Using docker image:

docker run --rm -it quay.io/lifebitaiorg/cloudos-cli:v2.12.0

Global variables:

MY_API_KEY="xxxx"
CLOUDOS="https://cloudos.lifebit.ai"
CLOUDOS2="https://cloudos.lifebit.ai/"
WORKSPACE_ID="xxxx"
PROJECT_NAME="davidp_testing"
WORKFLOW_NAME="Cufflinks"
WORKFLOW_NAME2="GWAS"
JOB_PARAMS="/cloudos/examples/rnatoy.config"

Normal workflow default job run

cloudos job run \
    --cloudos-url $CLOUDOS \
    --apikey $MY_API_KEY \
    --workspace-id $WORKSPACE_ID \
    --project-name "$PROJECT_NAME" \
    --workflow-name $WORKFLOW_NAME \
    --job-config $JOB_PARAMS \
    --job-name cloudos_cli_default
Screenshot 2024-11-27 at 21 00 43 Screenshot 2024-11-27 at 21 01 19

Normal workflow --nextflow-version=22.10.8

cloudos job run \
    --cloudos-url $CLOUDOS \
    --apikey $MY_API_KEY \
    --workspace-id $WORKSPACE_ID \
    --project-name "$PROJECT_NAME" \
    --workflow-name $WORKFLOW_NAME \
    --job-config $JOB_PARAMS \
    --nextflow-version 22.10.8 \
    --job-name cloudos_cli_22.10.8
Screenshot 2024-11-27 at 21 39 11 Screenshot 2024-11-27 at 21 05 01

Normal workflow --nextflow-version=24.04.4

cloudos job run \
    --cloudos-url $CLOUDOS \
    --apikey $MY_API_KEY \
    --workspace-id $WORKSPACE_ID \
    --project-name "$PROJECT_NAME" \
    --workflow-name $WORKFLOW_NAME \
    --job-config $JOB_PARAMS \
    --nextflow-version 24.04.4 \
    --job-name cloudos_cli_24.04.4
Screenshot 2024-11-27 at 21 06 43 Screenshot 2024-11-27 at 21 07 08

Normal workflow --nextflow-version=latest

cloudos job run \
    --cloudos-url $CLOUDOS \
    --apikey $MY_API_KEY \
    --workspace-id $WORKSPACE_ID \
    --project-name "$PROJECT_NAME" \
    --workflow-name $WORKFLOW_NAME \
    --job-config $JOB_PARAMS \
    --nextflow-version latest \
    --job-name cloudos_cli_latest
Screenshot 2024-11-27 at 21 44 50 Screenshot 2024-11-27 at 21 47 23

--nextflow-version=wrong.version

cloudos job run \
    --cloudos-url $CLOUDOS \
    --apikey $MY_API_KEY \
    --workspace-id $WORKSPACE_ID \
    --project-name "$PROJECT_NAME" \
    --workflow-name $WORKFLOW_NAME \
    --job-config $JOB_PARAMS \
    --nextflow-version wrong.version \
    --job-name cloudos_cli_wrong
Screenshot 2024-11-27 at 21 48 28

System tool default job

cloudos job run \
    --cloudos-url $CLOUDOS2 \
    --apikey $MY_API_KEY \
    --workspace-id $WORKSPACE_ID \
    --project-name "$PROJECT_NAME" \
    --workflow-name $WORKFLOW_NAME2 \
    --nextflow-profile "test_ci_staging_cloudos_phenofile" \
    --job-name cloudos_cli_module_default
Screenshot 2024-11-27 at 21 53 10 Screenshot 2024-11-27 at 21 53 35

System tool --nextflow-version=24.04.4

cloudos job run \
    --cloudos-url $CLOUDOS2 \
    --apikey $MY_API_KEY \
    --workspace-id $WORKSPACE_ID \
    --project-name "$PROJECT_NAME" \
    --workflow-name $WORKFLOW_NAME2 \
    --nextflow-profile "test_ci_staging_cloudos_phenofile" \
    --nextflow-version 24.04.4 \
    --job-name cloudos_cli_module_24.04.4
Screenshot 2024-11-27 at 21 58 23 Screenshot 2024-11-27 at 21 59 01

@dapineyro dapineyro marked this pull request as draft November 27, 2024 21:01
@dapineyro dapineyro marked this pull request as ready for review November 27, 2024 21:01
Copy link
Contributor

@danielboloc danielboloc left a comment

Choose a reason for hiding this comment

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

Great implementation! LGTM

Copy link
Member

@s0rthak s0rthak left a comment

Choose a reason for hiding this comment

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

Overall looks good to me. Good job 👍🏼

Added one code quality suggestion that can be implemented before merging and a wider logger improvement that we can explore in a different PR.

cloudos/__main__.py Show resolved Hide resolved
cloudos/queue/queue.py Show resolved Hide resolved
@dapineyro dapineyro merged commit 25d76d1 into main Nov 28, 2024
8 checks passed
@dapineyro dapineyro deleted the implements_nextflow_versions branch November 28, 2024 12:36
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.

3 participants