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

https and new curl fix #88

Merged
merged 4 commits into from
Jan 10, 2023
Merged

https and new curl fix #88

merged 4 commits into from
Jan 10, 2023

Conversation

Roleren
Copy link
Contributor

@Roleren Roleren commented Dec 21, 2022

Hello again, times are changing and https URLs is now starting to be enforced for many systems.

That being the case I fear biomartr will start to fail for many of the calls to older "http" URLs.
I here did a small fix for the most important links for ensemble genomes.
To keep biomartr alive, hopefully if enough people do some small fixes when needed it can continue to function properly.

Secondly, the newest curl-lib now enforces a trailing slash on URL directories, which made one of your FTP file checkers fail.
I made a safety wrapper for the dir check, that always enforces a single slash end.

The changes I made are completely backwards compatible for others, while making it not crash on people with newer systems.

I tested it on 2 systems, and ran your checks.

I see that your full checks (The ones ignored on CRAN) fails for several points, by giving https links I fixed some of them, but a new commit is needed to fix more of them.
Idealy some of your links should be moved out to a single function, so that if it ever changes in the future, there will be only 1 line to edit instead of a 100 which it is currently.

Especially this call:

jsonlite::fromJSON(
            "http://rest.ensembl.org/info/data/?content-type=application/json"
    )$releases

Which I changed to:

jsonlite::fromJSON(
            "https://rest.ensembl.org/info/data/?content-type=application/json"
    )$releases

It is used over 20 times, and it is far from the only one. So there is a lot of redundancy that could be moved to a single function :)

Merry Christmas :)

@HajkD
Copy link
Member

HajkD commented Jan 10, 2023

Hey Håkon,

Happy New Year! :-)

Thank you so much for your wonderful work again!!!!

This helps a lot and I agree that reducing the redundancy in biomartr is key to keeping it alive with all the changes happening.

I will put it on my TODO list and will be grateful for any insights from your side along the way!

With many thanks,
Hajk

@HajkD HajkD merged commit f5aea4b into ropensci:master Jan 10, 2023
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