ropensci / elastic

R client for the Elasticsearch HTTP API
https://docs.ropensci.org/elastic
Other
245 stars 58 forks source link

Bump to Milestone 1.3 #290

Open Banjio opened 2 years ago

Banjio commented 2 years ago

Dear Repository Maintainer,

I am in dire need need of the scroll functionality with ignore_versions = TRUE, which was fixed in milestone 1.3. Since the last change was made some time ago, would you mind upgrading the packages in CRAN to this version? In the meantime, I will build the package from the source code. If you need any help, I will gladly offer it to you.

Kind regards,

Banjio

sckott commented 2 years ago

Sure i'll try this week probably

Banjio commented 2 years ago

Thank you very much. I run into another problem with the function when building locally. I will come back at you tomorrow with the error cause and hopefully a potential fix.

Kind regards,

Banjio

Banjio commented 2 years ago

Hi sckott,

i had some time narrowing the issue down. Calling scroll with a connection where ignore_version=TRUE has the consequence that the parameters body and args are not set which are used in the call scroll_POST

scroll.character <- function(conn, x, time_scroll = "1m", raw = FALSE, asdf = FALSE,
                             stream_opts = list(), ...) {

  is_conn(conn)
  calls <- names(list(...))
  if ("scroll" %in% calls) {
    stop("The parameter `scroll` has been removed - use `time_scroll`")
  }  
  if (!conn$ignore_version){
    if (conn$es_ver() < 200) {
      body <- x
      args <- list(scroll = time_scroll)
    } else {
      body <- list(scroll = time_scroll, scroll_id = x)
      args <- list()
    }
  }
  tmp <- scroll_POST(
    conn = conn,
    path = "_search/scroll",
    args = args,
    body = body,
    raw = raw,
    asdf = asdf,
    stream_opts = stream_opts, ...)
  attr(tmp, "scroll") <- time_scroll
  return(tmp)
}

I am not certain what you are doing in scroll_POST but using this call

scrollReqBody = list("scroll"="1m","scroll_id"=res$`_scroll_id`)
req <-  httr::POST(url=paste0(conn$make_url(), "/_search/scroll"), 
                       body = scrollReqBody, encode="json", authenticate(conn$user, conn$pwd))

is successful. Thus it would probably work doing something like this

scroll.character <- function(conn, x, time_scroll = "1m", raw = FALSE, asdf = FALSE,
                             stream_opts = list(), ...) {

  is_conn(conn)
  calls <- names(list(...))
  if ("scroll" %in% calls) {
    stop("The parameter `scroll` has been removed - use `time_scroll`")
  }  
  if (!conn$ignore_version){
    if (conn$es_ver() < 200) {
      body <- x
      args <- list(scroll = time_scroll)
    } else {
      body <- list(scroll = time_scroll, scroll_id = x)
      args <- list()
    }
  } else {
    body <- list(scroll = time_scroll, scroll_id = x)
     args <- list()
}
  tmp <- scroll_POST(
    conn = conn,
    path = "_search/scroll",
    args = args,
    body = body,
    raw = raw,
    asdf = asdf,
    stream_opts = stream_opts, ...)
  attr(tmp, "scroll") <- time_scroll
  return(tmp)
}

It would be amazing if you could incorporate this behaviour before pushing the milestone 1.3 to cran. If you need any further insights don`t hesitate to contact me.

Kind regards,

Banjio

sckott commented 2 years ago

@Banjio can you install and see if it works for you - install from my fork remotes::install_github("sckott/elastic")

Banjio commented 2 years ago

Sorry sckott i totally forgot to do it and now i am in vacation with No Access to my Computer. I will do it First Thing upon Return.

Kind regards,

Banjio

Banjio commented 1 year ago

Hey sckott,

i am really sorry for the long wait. A lot of work related matter blocked me from testing and answering. But now i finally had the chance to test the updated version of the scroll api and it works fine now :). Thank you very much for your time and help.

Kind regards,

Banjio

sckott commented 1 year ago

great, glad it worked for you

maelle commented 1 year ago

For info this package is looking for a new maintainer cf #292 :smile_cat:

Banjio commented 1 year ago

Hey Maelle,

Sry for the late reply. I would be willing to step up as a maintainer with others. I feel not confident enough to maintain this repo alone. But if you find a team of maintainers I am willing to be part of it.

Kind regards

Banjio

maelle commented 1 year ago

:wave: @Banjio! Thank you! Could you please comment in #292 so that if someone else wants to be part of a team, they see your willingness to be involved? Thanks a ton and have a great week!