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

The Big Merge #12

Merged
merged 57 commits into from
Oct 17, 2023
Merged

The Big Merge #12

merged 57 commits into from
Oct 17, 2023

Conversation

DLBPointon
Copy link
Contributor

Closes #6

Release ready pipeline!

@DLBPointon
Copy link
Contributor Author

@DLBPointon Do you want to fix the CI tests in this pipeline too?

I will be. I'll ask Guoying about it tomorrow, as it's all tower stuff, we are in an ascc Hackathon at the minute.

@gq1
Copy link
Member

gq1 commented Sep 27, 2023

The tower agent was down two days ago. I re-run the job and submitted it to Tower successfully.

But the pipeline failed to run.
https://tower.nf/orgs/sanger-tol/workspaces/github-ci/watch/4Mp053V8l2TU1H


ERROR ~ ERROR: Validation of pipeline parameters failed!

 -- Check 'nf-4Mp053V8l2TU1H.log' file for details
ERROR ~ * --input: string [/lustre/scratch123/tol/resources/treeval/treeval-testdata/TreeValSmallData/Oscheius_DF5033/assembly/draft/DF5033.hifiasm.noTelos.20211120/DF5033.noTelos.hifiasm.purged.noCont.noMito.fasta] does not match pattern ^\S+\.fa$ (/lustre/scratch123/tol/resources/treeval/treeval-testdata/TreeValSmallData/Oscheius_DF5033/assembly/draft/DF5033.hifiasm.noTelos.20211120/DF5033.noTelos.hifiasm.purged.noCont.noMito.fasta)

 -- Check 'nf-4Mp053V8l2TU1H.log' file for details

Exiting!

@DLBPointon
Copy link
Contributor Author

The tower agent was down two days ago. I re-run the job and submitted it to Tower successfully.

But the pipeline failed to run. https://tower.nf/orgs/sanger-tol/workspaces/github-ci/watch/4Mp053V8l2TU1H


ERROR ~ ERROR: Validation of pipeline parameters failed!

 -- Check 'nf-4Mp053V8l2TU1H.log' file for details
ERROR ~ * --input: string [/lustre/scratch123/tol/resources/treeval/treeval-testdata/TreeValSmallData/Oscheius_DF5033/assembly/draft/DF5033.hifiasm.noTelos.20211120/DF5033.noTelos.hifiasm.purged.noCont.noMito.fasta] does not match pattern ^\S+\.fa$ (/lustre/scratch123/tol/resources/treeval/treeval-testdata/TreeValSmallData/Oscheius_DF5033/assembly/draft/DF5033.hifiasm.noTelos.20211120/DF5033.noTelos.hifiasm.purged.noCont.noMito.fasta)

 -- Check 'nf-4Mp053V8l2TU1H.log' file for details

Exiting!

Hi Guoying,
Thanks for having a look, I've fixed the issue in: #13
The Schema just needed updating, I also changed the outdir value other wise it would constantly come back as null in tests.

@gq1
Copy link
Member

gq1 commented Sep 28, 2023

Hi Guoying, Thanks for having a look, I've fixed the issue in: #13 The Schema just needed updating, I also changed the outdir value other wise it would constantly come back as null in tests.

Can you re-trigger the Github action run to test?

@DLBPointon
Copy link
Contributor Author

Morning @priyanka-surana, @gq1
Looks like all tests including Tower have been completed successfully.

@DLBPointon
Copy link
Contributor Author

Hi @priyanka-surana,
Can this now be approved?

@DLBPointon
Copy link
Contributor Author

Hi @priyanka-surana, Can this now be approved?

Never mind this should wait on #14

Copy link
Contributor

@priyanka-surana priyanka-surana left a comment

Choose a reason for hiding this comment

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

I tried the test profile and it fails for me.

nextflow run . -profile test,singularity --outdir results -ansi-log false
ERROR ~ Unable to read script: '/lustre/scratch123/tol/teams/tolit/users/ps22/pipelines/sanger-tol-curationpretext_dev/dev/./workflows/curationpretext_allf.nf' -- cause: /home/runner/work/curationpretext/curationpretext/TreeValTinyData/genomic_data/pacbio

I can see the test profile is set for GitHub, which profile should I be using to test the subset and full datasets? Thanks.

conf/base.config Outdated Show resolved Hide resolved
@gq1
Copy link
Member

gq1 commented Oct 3, 2023

I tried the test profile on the farm as well and got the same error message.

Can we have the instruction as @muffato did to TreeVal to have some instructions to download the test file from S3 and replace all the github path to the local path?

I can see the test profile is set for GitHub, which profile should I be using to test the subset and full datasets? Thanks.

I also tried the test_full profile directly on the farm, it works fine. This profile already passed on the farm via Github action and Tower.

@DLBPointon
Copy link
Contributor Author

Thanks both @gq1 and @priyanka-surana.
I've submitted fixes, they are on #14. usage now contains information on amending the test.config.

@DLBPointon
Copy link
Contributor Author

#14 is now merged

Copy link
Contributor

@priyanka-surana priyanka-surana left a comment

Choose a reason for hiding this comment

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

LGTM 👍🏼

@DLBPointon DLBPointon merged commit 661b2ce into main Oct 17, 2023
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants