feat: add httr2_translate#797
Conversation
|
CI has been tamed. Please let me know if there is anything I can do to move this along! I'd quite like to be able to use it as a debug option in if (isTRUE(getOption("arcgis.debug_curl")) {
cat(httr2::httr2_translate(req))
}This will help tremendously in getting reproducible examples from users as well as providing them to our distributed team of software engineers. Thanks! |
| if (length(cmd_parts) <= 2) { | ||
| paste(cmd_parts, collapse = " ") | ||
| } else { |
There was a problem hiding this comment.
Is this branch actually necessary?
There was a problem hiding this comment.
This branch is used so that the output is compatible with curl_translate() if curl is on its own line curl_translate() fails to read the result.
There was a problem hiding this comment.
I understand that but I think if remove the first branch then the results will still be the same.
I'd suggest renaming and refocussing cmd_parts to curl_args (i.e. adding curl and the url at the very end) to make this more clear.
|
Thank you so much for the feedback! I'll review this and make the requested changes. |
| headers <- req_get_headers(req, redacted = "reveal") | ||
|
|
||
| # if headers are present, add them using -H flag | ||
| if (!rlang::is_empty(headers)) { |
There was a problem hiding this comment.
Again, you don't need to first check if the vector is empty, if it is empty the for loop already doesn't do anything.
| } | ||
| } | ||
|
|
||
| known_curl_opts <- c( |
There was a problem hiding this comment.
I'd suggest making this a separate function
| # if the headers aren't empty AND the content-type header is set | ||
| # we use that instead of what is inferred from the request object | ||
| if ( | ||
| !rlang::is_empty(headers) && ("content-type" %in% tolower(names(headers))) |
There was a problem hiding this comment.
If x is empty, then "foo" %in% x will always return FALSE.
…nslate # Conflicts: # _pkgdown.yml
- Extract option translation into req_options_as_curl() so it can be tested in isolation - Drop redundant is_empty() guards before the headers loop and the content-type %in% check - Fix misplaced @Seealso so it renders as its own section Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Restructure into small parallel helpers (req_method_as_curl(), req_headers_as_curl(), req_options_as_curl(), req_body_as_curl()) that each return a character vector of curl arguments, assembled by a single curl_command() formatter. This removes the cmd_args accumulation, the manual quote-building (now dquote()), and the multi-line special-casing. - Add `obfuscated` argument (matching req_get_body()), passed through to both header and body extraction; secrets are redacted by default. - Upgrade the untranslatable-option alert to a real cli_warn(). - Avoid emitting a duplicate Content-Type when set as a request header. - Reach 100% coverage with unit tests for each helper plus the raw-body and Content-Type cases. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This reverts commit f9e92f4.
|
Sorry for letting this die, and thank you so much for bringing it back <3 |
|
No problems. FWIW I switched back to |
2 participants
This PR closes #795 by adding an
httr2_translate()function.This function takes an
httr2_requestobject and does its best to faithfully create the equivalent curl request while respecting headers, cookies, options, request type etc.Created on 2025-08-20 with reprex v2.1.1