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

Don't check URLs in ignore list #24

Merged
merged 2 commits into from
Jan 26, 2024

Conversation

tmooney
Copy link
Contributor

@tmooney tmooney commented Jul 25, 2023

I was running into an issue with the url-check.R script. The course site I am building contains links to some very large files, which cause the checker to fail. When I added those URLs to the ignore-urls.txt, the script still failed since it tries to GET the link (twice!) before throwing the results of the check away.

This PR re-uses the status from the first GET call and also checks the ignore list inside of test_url() reporting back a status of ignored instead of success or failure. With these changes I was able to run the script on my project and have it report properly.

That result is still filtered out before the report is made; I could imagine also removing the dplyr::filter(!(urls %in% ignore_urls)) from the end of the script since they still won't be included by the failed filter as is.

Copy link
Contributor

@cansavvy cansavvy left a comment

Choose a reason for hiding this comment

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

@tmooney Thanks so much for this add!

This is great!

@cansavvy cansavvy merged commit 0c7e75f into jhudsl:main Jan 26, 2024
tmooney added a commit to griffithlab/Immuno_Workflow_Course that referenced this pull request Jan 29, 2024
Now that jhudsl/ottr-reports#24 is merged, we should be able to turn
this on without having it attempt to download the FASTQ tarballs each
time.
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