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

First attempt to make download work with new DSL2 syntax. #1385

Merged
merged 2 commits into from
Jan 20, 2022

Conversation

ewels
Copy link
Member

@ewels ewels commented Jan 19, 2022

Update the code-scraping-code that discovers container URLs in DSL2 module code. Hopefully now works with both old and new DSL2 styles. Should close #1379.

PR checklist

  • This comment contains a description of changes (with reason)
  • CHANGELOG.md is updated
  • If you've fixed a bug or added code that should be tested, add tests!
  • Documentation in docs is updated

@ewels ewels requested a review from drpatelh January 19, 2022 14:15
@ewels
Copy link
Member Author

ewels commented Jan 19, 2022

Opening this a little early as I'm still technically ill and not working, but want to get this moving ASAP.
⚠️ Currently completely untested.

Any help with testing much appreciated. Apologies in advance for dumb errors. I'm going to go and take a nap now. 🤒 😴

@ewels
Copy link
Member Author

ewels commented Jan 19, 2022

Ok I lied, I didn't go to bed yet and did some very quick testing to make sure that it runs (it didn't).

Speed test looks good at my end now:

✅ Finds 28 containers for nf-core/rnaseq version 3.2 (old DSL2 syntax)
✅ Finds 30 containers for nf-core/rnaseq version 3.5 (new DSL2 syntax)
✅ Finds 1 non-https container for nf-core/methylseq (DSL1)

More testing by others would be much appreciated 👍🏻

@ewels ewels marked this pull request as ready for review January 19, 2022 14:34
@codecov

This comment has been minimized.

if matches:
for match in matches:
# Look for a http download URL.
# Thanks Stack Overflow for the regex: https://stackoverflow.com/a/3809435/713980
Copy link
Member

Choose a reason for hiding this comment

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

🙇🏽

for match in matches:
# Look for a http download URL.
# Thanks Stack Overflow for the regex: https://stackoverflow.com/a/3809435/713980
url_regex = r"https?:\/\/(www\.)?[-a-zA-Z0-9@:%._\+~#=]{1,256}\.[a-zA-Z0-9()]{1,6}\b([-a-zA-Z0-9()@:%_\+.~#?&//=]*)"
Copy link
Member

Choose a reason for hiding this comment

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

🤯

Copy link
Member

@drpatelh drpatelh left a comment

Choose a reason for hiding this comment

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

I haven't had time to test it I'm afraid but if you think it's good to go then let's get it in. I will test when I get a moment.

@ewels ewels merged commit ffa9c9a into nf-core:dev Jan 20, 2022
@ewels ewels deleted the new-dsl2-singularity-syntax-dl branch January 20, 2022 14:29
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