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

efforts for using the job id for various utility functions #328

Merged
merged 12 commits into from Mar 4, 2017

Conversation

jordansread
Copy link
Contributor

Fixes #236

@jordansread
Copy link
Contributor Author

this PR makes it possible to do

check("https://cida.usgs.gov:443/gdp/process/RetrieveResultServlet?id=5decb04b-056d-4c46-a2c0-13908711a3a7")
successful("https://cida.usgs.gov:443/gdp/process/RetrieveResultServlet?id=5decb04b-056d-4c46-a2c0-13908711a3a7")
error("https://cida.usgs.gov:443/gdp/process/RetrieveResultServlet?id=5decb04b-056d-4c46-a2c0-13908711a3a7")
download("https://cida.usgs.gov:443/gdp/process/RetrieveResultServlet?id=5decb04b-056d-4c46-a2c0-13908711a3a7")
result("https://cida.usgs.gov:443/gdp/process/RetrieveResultServlet?id=5decb04b-056d-4c46-a2c0-13908711a3a7")

and the like.

one thing it doesn't do is let you pick up a job ID and start a wait until finished checker on it. But that wasn't part of this task as far as I understood it.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 04cc6da on jread-usgs:master into ** on USGS-R:master**.

@wdwatkins wdwatkins closed this Mar 3, 2017
@wdwatkins wdwatkins reopened this Mar 3, 2017
@wdwatkins
Copy link
Contributor

oops 😁

@jordansread
Copy link
Contributor Author

Just added the wait methods on ID for completeness.

job <- geoknife(stencil = c(-89,42), fabric = 'prism', wait=FALSE)
wait(id(job)) %>% result()
     DateTime bufferedPoint variable statistic
1  1895-01-01       41.1600      ppt      MEAN
2  1895-02-01       13.9825      ppt      MEAN
 ...

where id(job) is the process URL I refer to above ☝️

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling e99cd31 on jread-usgs:master into ** on USGS-R:master**.

Copy link
Contributor

@jiwalker-usgs jiwalker-usgs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

#'@param destination a file destination. If missing, a temp directory will be used
#'@param ... additional arguments passed to \code{\link[httr]{write_disk}}, such as overwrite = TRUE
#'@return the file handle
#'@return the destination of the downloaded file
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since the file is downloaded, this should be location rather that destination, no?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you are right. Changing.


#' @rdname wait
#' @aliases wait
setMethod(f = "wait",signature(.Object = "character", sleep.time = "missing"), definition = function(.Object, sleep.time){
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if there is a way to do this without needed 4 signatures? If we add any more arguments the number of signatures keeps increasing.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We only need to define types for signatures that we need to dispatch based on. Alternatively, I could use "ANY" for the sleep.time signature (or leave that off) and handle if(missing(sleep.time)) within the function

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See https://github.com/USGS-R/geoknife/blob/master/R/successful.R#L37 for example - only dispatching based on the first arg, so I can leave off signatures for the second arg

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

... but now I see that the pattern I linked to isn't actually using that default because of the way s4 works (which is why I used "missing" here)...oh boy

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will clean up what I just discovered @jiwalker-usgs ☝️

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling b497cf3 on jread-usgs:master into ** on USGS-R:master**.

@jordansread jordansread merged commit a67dafa into DOI-USGS:master Mar 4, 2017
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

Successfully merging this pull request may close these issues.

None yet

4 participants