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.

Sign up for GitHub

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

Jump to bottom

Tower #142

Merged
merged 14 commits into from
Sep 21, 2023
Merged

Tower #142

merged 14 commits into from
Sep 21, 2023

Conversation

DLBPointon
Copy link
Contributor

@DLBPointon DLBPointon commented Sep 19, 2023 •
edited
Loading

Adding sanger tower integration files.

@gq1 has suggested we default FULL in order to allow sanger testing to work. This PR does that and adds all the required files.

@DLBPointon DLBPointon added enhancement New feature or request Release 1 Issues required for first release labels Sep 19, 2023
@DLBPointon DLBPointon requested review from muffato and gq1 September 19, 2023 15:39
@DLBPointon DLBPointon self-assigned this Sep 19, 2023
@github-actions
Copy link

github-actions bot commented Sep 19, 2023 •
edited
Loading

nf-core lint overall result: Passed ✅ ⚠️

Posted for pipeline commit 718acec

+| ✅ 126 tests passed       |+
#| ❔  17 tests were ignored |#
!| ❗   8 tests had warnings |!

❗ Test warnings:

  • files_exist - File not found: conf/igenomes.config
  • files_exist - File not found: .github/workflows/awstest.yml
  • files_exist - File not found: .github/workflows/awsfulltest.yml
  • nextflow_config - Config manifest.version should end in dev: '1.0.0'
  • readme - README contains the placeholder zenodo.XXXXXXX. This should be replaced with the zenodo doi (after the first release).
  • pipeline_todos - TODO string in WorkflowMain.groovy: Add Zenodo DOI for pipeline after first release
  • pipeline_todos - TODO string in methods_description_template.yml: ## Update the HTML below to your prefered methods description, e.g. add publication citation for this pipeline
  • system_exit - System.exit in WorkflowTreeval.groovy: System.exit(1) [line 17]

❔ Tests ignored:

  • files_exist - File is ignored: assets/nf-core-treeval_logo_light.png
  • files_exist - File is ignored: conf/test_full.config
  • files_exist - File is ignored: docs/images/nf-core-treeval_logo_light.png
  • files_exist - File is ignored: docs/images/nf-core-treeval_logo_dark.png
  • nextflow_config - Config variable ignored: manifest.name
  • nextflow_config - Config variable ignored: manifest.homePage
  • files_unchanged - File ignored due to lint config: LICENSE or LICENSE.md or LICENCE or LICENCE.md
  • files_unchanged - File ignored due to lint config: .github/CONTRIBUTING.md
  • files_unchanged - File ignored due to lint config: .github/ISSUE_TEMPLATE/bug_report.yml
  • files_unchanged - File ignored due to lint config: .github/workflows/linting.yml
  • files_unchanged - File ignored due to lint config: assets/sendmail_template.txt
  • files_unchanged - File does not exist: assets/nf-core-treeval_logo_light.png
  • files_unchanged - File does not exist: docs/images/nf-core-treeval_logo_light.png
  • files_unchanged - File does not exist: docs/images/nf-core-treeval_logo_dark.png
  • files_unchanged - File ignored due to lint config: lib/NfcoreTemplate.groovy
  • files_unchanged - File ignored due to lint config: .gitignore or .prettierignore or pyproject.toml
  • actions_awstest - 'awstest.yml' workflow not found: /home/runner/work/treeval/treeval/.github/workflows/awstest.yml

✅ Tests passed:

Run details

  • nf-core/tools version 2.8
  • Run at 2023-09-21 09:58:47

@gq1
Copy link
Member

gq1 commented Sep 19, 2023

This is fine. The only problem I have is the profile names.

In all other pipelines, we use test and test_full as the profile name, instead yours github_test and farm_test

https://github.com/sanger-tol/insdcdownload/tree/dev/conf

In one of my following jobs, I only archive the test_full to results to S3,

@gq1
Copy link
Member

gq1 commented Sep 19, 2023

I can't see any changes for the main.nf, how can you make the full workflow as the default workflow?

@DLBPointon
Copy link
Contributor Author

I can't see any changes for the main.nf, how can you make the full workflow as the default workflow?

I just realised I didn't upload the changes in that file, they are there now. Just needed to remove the name from workflow FULL {}

@DLBPointon
Copy link
Contributor Author

This is fine. The only problem I have is the profile names.

In all other pipelines, we use test and test_full as the profile name, instead yours github_test and farm_test

https://github.com/sanger-tol/insdcdownload/tree/dev/conf

In one of my following jobs, I only archive the test_full to results to S3,

I'll rename it for consistency.

@gq1
Copy link
Member

gq1 commented Sep 19, 2023 •
edited
Loading

Are the profile test and github_test the same? Where are the test data? We can put the data into our S3 and then this profile should work anywhere.

I can see the difference between these 2 profiles now, they are the same. But when you run test profile on the farm through Tower, you also need to download the test data first. You need config the paths for the data as well.

But you have another profile with _local using the testing data on nfs.

You don't need the sanger_test workflow if you don't want to trigger your pipeline test on the farm manully.

Can you check where is the test data? We normally have 2 versions of test data, simple test data on S3, full test data on lustre.

When and where you want to run which test?

@DLBPointon
Copy link
Contributor Author

They are the same, I have fixed that now.

What we want is small testing to be performed on GitHub, which we now have, and large genome testing on the farm. So this would be using test_full in the sanger_test_full.config wouldn't it? The current full test data is on lustre already in the test-data directory (we'll need to update that data to something bigger in the near future).

For test_full we can just use lustre path? If we use S3 for the full testing then we will need to package up the entire test directory because of how we have things set up, like we do with github testing at the minute.

I've deleted the sanger_test.config as I don't think we'll need the manual testing.

The local_github_testing was just so that I could test everything the whole github CI but locally. I can delete that now.

So for now, I just need to fix test_full for lustre paths to out large test set?

@DLBPointon
Copy link
Contributor Author

Working with Guoying yesterday, we have finalised everything needed for testing the pipeline on the Farm.

Waiting on final approval now.

@gq1
Copy link
Member

gq1 commented Sep 21, 2023

I tried to run your pipeline on the farm and got one error here:

nextflow run treeval/ -profile test_full,sanger,singularity,cleanup --outdir test_results
N E X T F L O W  ~  version 23.04.1
ERROR ~ No such file or directory: Config file does not exist: /lustre/scratch123/tol/teams/tolit/users/gq2/git_test/treeval/conf/farm_test.config

The config file not updated here:

treeval/nextflow.config

Line 156 in 8e8f64b

test_full { includeConfig 'conf/farm_test.config' }

@DLBPointon DLBPointon merged commit 7fc4137 into dev Sep 21, 2023
6 checks passed
@DLBPointon DLBPointon deleted the tower branch September 21, 2023 12:00
@muffato muffato added this to the Release 1 milestone Sep 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gq1 gq1 gq1 approved these changes

@muffato muffato Awaiting requested review from muffato

@priyanka-surana priyanka-surana Awaiting requested review from priyanka-surana

Assignees

@DLBPointon DLBPointon

Labels
enhancement New feature or request Release 1 Issues required for first release
Projects
None yet
Milestone
Release 1
Development

Successfully merging this pull request may close these issues.
3 participants
@DLBPointon @gq1 @muffato