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

Bump to Milestone 1.3 #290

Open
Banjio opened this issue May 24, 2022 · 10 comments
Open

Bump to Milestone 1.3 #290

Banjio opened this issue May 24, 2022 · 10 comments

Comments

@Banjio
Copy link
Collaborator

Banjio commented May 24, 2022

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
Copy link
Contributor

sckott commented May 24, 2022

Sure i'll try this week probably

@Banjio
Copy link
Collaborator Author

Banjio commented May 24, 2022

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
Copy link
Collaborator Author

Banjio commented May 25, 2022

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
Copy link
Contributor

sckott commented Jun 1, 2022

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

@Banjio
Copy link
Collaborator Author

Banjio commented Jun 14, 2022

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
Copy link
Collaborator Author

Banjio commented Aug 15, 2022

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
Copy link
Contributor

sckott commented Aug 15, 2022

great, glad it worked for you

@maelle
Copy link
Member

maelle commented Sep 9, 2022

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

@Banjio
Copy link
Collaborator Author

Banjio commented Oct 7, 2022

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
Copy link
Member

maelle commented Oct 10, 2022

👋 @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!

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

No branches or pull requests

3 participants