Replace RCurl with curl

[infotroph]

The RCurl package is old, not really maintained, and doesn't handle authentication well. The newer curl package is well-designed, arguably easier to use, and likely reduces security holes. I haven't tested yet, but on a quick glance it looks like the conversions would be easy to do.

[shivanshu1086]

passing
I am kind of doing in it something as R has an official package for Curl https://cran.r-project.org/web/packages/curl/curl.pdf.
I tried into it but it is showing error for symbols

, can you please look at it.

[ashiklom]

I think you are confusing shell (e.g. bash) and R syntax here: curl -X GET... is how you would do this in the shell, but the R interface is pretty different. I would suggest you carefully review the examples on the curl package vignette to familiarize yourself with the correct usage and syntax.

[github-actions]

This issue is stale because it has been open 365 days with no activity.

[infotroph]

@moki1202 might you be interested in this as a next project? The general approach in outline:

• Make a list of the places in PEcAn RCurl is used. I would do this by grepping the entire directory for the string RCurl, but other approaches are definitely possible.
• Make a list of which functions from the RCurl package we use and which functions from the curl package to use to replace each one.
• Optional challenge level: Determine which of our existing usages are covered by unit tests, and write new tests to improve coverage if needed.
• Change all calls over to use curl
• Update dependency declarations in each package's DESCRIPTION file
• Test, merge, and close this issue!

[moki1202]

@infotroph thankyou . ill start working on it.

[moki1202]

@infotroph @ashiklom so I gave a thorough read to both these packages and did manage to find replacement functions for some of them. like getURL could be replaced with curl_download, ( postform - form_file), ( fileupload - curl_upload) but that's it . after checking all the functions in curl I couldn't find the replacement calls for " url.exists, CFILE, curlperform, close, basicTextGatherer)".
it seemed to me like curl package doesn't have enough functions as RCurl. some help would be greatly appreciated here.

[infotroph]

Getting this far is a valuable contribution already! I think we can probably find replacements for all of these. Thoughts from a very brief look at your list:

• url.exists is shorthand for "make an http HEAD request and report whether the status is success". httr::HEAD might be the most obvious replacement, but if we want to stay in pure curl it Looks like we can do header-only requests with something like the rather non-obvious curl::curl_get_memory(url, handle = curl::new_handle(nobody = 1)). I also see that curl::curl_download performs a status check before starting the download anyway, so some existing url.exists usages might be redundant once we update the download calls they were protecting.
• CFILE/curlPerform/close looks like an open/stream/close pattern. I bet this can be replaced with the curl::curl streaming interface, but check me on that.
• I don't immediately know how to replace basicTextGatherer, but that doesn't mean it can't be done. Additionally, it's only used in one function that arguably reinvents the functionality of soilDB::SDA_query, so it's possible we could eliminate its use with a more ambitious refactor. Meanwhile, cutting our use of RCurl down to just one function would be major progress!

[infotroph]

Hah, I should have looked at the whole file before speaking. The two places we use CFILE (once in download.url and once in download.browndog, both look the same:

  f <- RCurl::CFILE(file, mode = "wb")
  RCurl::curlPerform(url = url, writedata = f@ref, .opts = .opts)
  RCurl::close(f)

So the "streaming" is really just one download operation anyway. I think this can probably be replaced with a simple curl::curl_download(url = url, destfile = file, handle = curl::new_handle(.list = .opts)), but we'll need to check for differences in the required naming/formatting of .opts.

[moki1202]

html <- RCurl::postForm(url, fileData = RCurl::fileUpload(zipfile), .opts = curloptions) if i wanted to change the calls here over to curl calls , I could use form_file function in place of postForm but that would require me to get rid of .opts=curloptions as form_file has 2 arguments i.e. ' path and type' . @infotroph am I right here or is there any other way to switch ?

[infotroph]

I think the best replacement forpostForm may be httr::POST, but would be interested in opinions from others.

[infotroph]

I'm not sure I follow your last question, but we've gotten far enough that it'll be easier to discuss with actual code! Go ahead and start a PR and we can iterate there.

[moki1202]

@infotroph can I assume that the use of ' test_that() ' function indicates the use of unit tests wherever applied?

[infotroph]

@moki1202 Yes, that's correct. The sneaky part is that many of our packages call testthat but only test a couple of the functions in the package, so there's lots of room for improvement.