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.

Sign up for GitHub

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

Jump to bottom

regression: req_perform_parallel() no longer works with form encoded data #549

Closed
JosiahParry opened this issue Sep 16, 2024 · 6 comments · Fixed by #551
Closed

regression: req_perform_parallel() no longer works with form encoded data #549

JosiahParry opened this issue Sep 16, 2024 · 6 comments · Fixed by #551

Comments

@JosiahParry
Copy link

Between httr2 1.0.1 and httr2 1.0.4 there appears to have been a regression with the way that form encoded data is handled with req_perform_parallel().

It was discovered in R-ArcGIS/arcgislayers#219

The below will work if you install httr2 v1.0.1 but not with 1.0.4.

library(httr2)
furl <- "https://services1.arcgis.com/Hp6G80Pky0om7QvQ/arcgis/rest/services/Public_Schools/FeatureServer/0/query"

req_pbf <- request(furl) |> 
  req_body_form(f = "pbf", where = "1=1", outFields = "*")

# works
req_perform(req_pbf)
#> <httr2_response>
#> POST
#> https://services1.arcgis.com/Hp6G80Pky0om7QvQ/arcgis/rest/services/Public_Schools/FeatureServer/0/query
#> Status: 200 OK
#> Content-Type: application/x-protobuf
#> Body: In memory (46535033 bytes)

# does not work
resps <- req_perform_parallel(list(req_pbf))
resps
#> [[1]]
#> <httr2_response>
#> POST
#> https://services1.arcgis.com/Hp6G80Pky0om7QvQ/arcgis/rest/services/Public_Schools/FeatureServer/0/query
#> Status: 200 OK
#> Content-Type: text/html
#> Body: In memory (10273 bytes)

substr(resp_body_string(resps[[1]]), 1, 1000)
#> [1] "<html lang=\"en\">\r\n<head>\r\n  <title>Query: Public_Schools (ID: 0)</title>\r\n  <link href='/ESRI.ArcGIS.SDS.REST.css' rel='stylesheet' type='text/css'>\r\n  <link rel='SHORTCUT ICON' href='/favicon.ico'>\r\n<meta http-equiv=\"Content-Type\" content=\"text/html; charset=UTF-8\"/> \r\n</head>\r\n<body>\r\n<table width=\"100%\" id=\"userTable\" class=\"userTable\">\r\n<tr>\r\n<td class=\"titlecell\">ArcGIS REST Services Directory</td>\r\n</tr>\r\n</table>\r\n<table id=\"navTable\" class=\"navTable\" width=\"100%\">\r\n<tbody>\r\n<tr valign=\"top\">\r\n<td class=\"breadcrumbs\">\r\n<span id=\"homeBreadcrumbs\"><a href=\"/Hp6G80Pky0om7QvQ/ArcGIS/rest/services\">Home</a> > <a href=\"/Hp6G80Pky0om7QvQ/ArcGIS/rest/services\">services</a> </span>> <a href=\"/Hp6G80Pky0om7QvQ/ArcGIS/rest/services/Public_Schools/FeatureServer\">Public_Schools (FeatureServer)</a> > <a href=\"/Hp6G80Pky0om7QvQ/ArcGIS/rest/services/Public_Schools/FeatureServer/0\">Public_Schools</a> > <a href=\"/Hp6G80Pky0om7QvQ/ArcGIS/rest/services/Public_Schools/FeatureServer/0/query\"><i>query"
@JosiahParry JosiahParry mentioned this issue Sep 16, 2024
Open
@JosiahParry
Copy link
Author

I've also noticed that this is affecting the {arcgisgeocode} package which also utilizes the req_perform_parallel() function with form body data.
https://cran.r-project.org/web/checks/check_results_arcgisgeocode.html

@JosiahParry JosiahParry mentioned this issue Sep 17, 2024
Closed
hadley added a commit that referenced this issue Sep 17, 2024
@hadley
Use prepared request in Performance
f72044f 
This ensures that the method and body are correctly set for `req_perform_parallel()` and `req_perform_promise()`. Fixes #549
@hadley hadley mentioned this issue Sep 17, 2024
Merged
@hadley
Copy link
Member

hadley commented Sep 17, 2024

Sorry about that 😞. I'll give it a couple more days in case I broke anything else and then I'll submit a fix to CRAN on Friday.

@JosiahParry
Copy link
Author

Totallyyy okay! I’ve got much bigger CRAN fish to fry at the moment. :) I’ve got until 2024-09-26 to submit a new version anyways!

hadley added a commit that referenced this issue Sep 17, 2024
@hadley
Use prepared request in Performance (#551)
9ae22b7 
This ensures that the method and body are correctly set for `req_perform_parallel()` and `req_perform_promise()`. Fixes #549
@JosiahParry
Copy link
Author

I'm curious if you have a suggestion on how to specify the required version of httr2 in our DESCRIPTION files. Previously it was specified as httr2 (>= 1.0.0). But now we know that 1.0.4 will not work but 1.0.5 will.

I would change to httr2 (>= 1.0.5) but I need to submit to cran by thursday to prevent removal.

@hadley
Copy link
Member

hadley commented Sep 24, 2024

I'll just release httr2 now to make things easier 😄

@JosiahParry
Copy link
Author

Lol! Thank you! Sorry!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Use prepared request in Performance r-lib/httr2
2 participants
@hadley @JosiahParry