maybe useful for advanced users.

Hi @sbgtengfei and @road2stat ,

Everything else in the Easy API seems to work that I have tried. The partial file name search is very powerful and great feature.

Re: upload...I followed this code

fl <- system.file("extdata", "sample1.fastq", package = "sbgr")
## create meta data
fl.meta <- list(file_type = "fastq",
                seq_tech = "Illumina",
                sample = "sample1",
                author = "tengfei")

proj$upload(fl, metadata = fl.meta)

error:

Error in validObject(.Object) : 
  invalid class “FileTypeSingleEnum” object: superclass "Enum" not defined in the environment of the object's class

Thanks!

Dave

new developer hub address
https://docs.sbgenomics.com/

need to update doc link for reference

Nan and Tengfei,

Overall this looks like a well written straightforward package that I would
gladly accept into Bioconductor with the below suggestions. Thank for your
patience with the review process and I apologize for the delay in reviewing
this package.

• Rather than supplying a token argument for every function (and checking if it
  exists) consider writing an authenticate() function, which stores the token
  in an environment within the package and a corresponding get function which
  will provide an appropriate error message if no token is provided. i.e.

.token <- new.env(parent = emptyenv())

get_token <- function() {
  token <- .token$token
  if (is.null(token)) {
    stop("Please authenticate with ",
        sQuote("authenticate()"),
        " or provide an ",
        sQuote("auth_token"))
  }
}

#' @export
authenticate <- function(auth_token) {

  # maybe make a request to check if the token is valid here?

  .token$token <- auth_token
}

# example for upload_info
upload_info = function (auth_token = get_token(), upload_id = NULL) {

    req = sbgapi(auth_token = auth_token,
                 path = paste0('upload/multipart/', upload_id),
                 method = 'GET')

    status_check(req)
}

• Explicit returns at the end of functions are superfluous (and actually slower),
  so it is best to just return the object directly. The only time you need
  an explicit return is when returning before the function's end.
• if a parameter can take a enumerated number of values I would suggest you use
  match.args() and a switch statement. This will provide you with error
  checking of the arguments and partial matching, and make the code more clear.

e.g.

sbgapi <- function(method = c('GET', 'POST', 'PUT', 'DELETE')) {

    method <- match.args(method)

    switch(method,
      GET = {
         GET()
      },
      POST = {
        #validation code
        POST()
      }
    )
}

In addition for the sbgapi() function because every method returns a request
response and the switch is the last statement in the function you don't need to
save them to a variable at all, just use the implicit return from the switch.

• A number of the files contained trailing whitespace characters
  throughout the project. While these characters do not affect the execution of
  the code, they are unnecessary. You can use the following one liner to strip them
  out find . -type f -not -path './.git/*' -exec perl -i -pe 's/ +$//' {} \;.
  I would also suggest turning on visible whitespace in your editor of choice
  so you can easily spot this in the future.
• In file_meta_update() You use if (is.null(project_id) | is.null(file_id)),
  when doing logical comparisons of 1 element vectors it is better to use ||
  rather than the vectorized |.

Thank you again for the submission and your patience. Let me know if you have
any further questions at (https://tracker.bioconductor.org/issue1264). I have
cross posted this response to your GitHub Issue tracker for convenience
(#15).

-Jim

so can easily switch to CGC and other API calles

The vignette link in readme is broken.

so developers can get full information from api call
for example

response <- function(x){
attr(x, "response")
}
status_check <- function (req)
{
if (status_code(req) %in% c("200", "201", "204")) {
res <- content(req, "parsed")
attr(res, "response") <- req
return(res)
}
else if (status_code(req) %in% c("401", "403", "404", "503")) {
msg = content(req, "parsed")$message
stop(paste0("HTTP Status ", status_code(req), ": ", msg),
call. = FALSE)
}
else {
stop("Error of unknown type occured")
}
}

need to figure out it should be API update or client side update

because it require credentials to test the API, let's just regularly run the tutorial or any testing script with our own credentials to make sure API returns what we expect. Even better if we could figure out an automated testing method.

define *List

Better representation and easier access, define show method for new objects, instead of printing list, print in a nicer format.

Could use S4 or R5 objects.

Create higher level friendly API, update tutorial with this wrapper API calls.

@road2stat in your test, is the upload return a 'etag' in the headers?

otherwise if body is empty, is.list(NULL) return FALSE

so we can add more headers

so pipeline information could be ported to input/output or workflow/tool object?