-
Notifications
You must be signed in to change notification settings - Fork 38
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
Added ds_upsert function and extended default parameters in ds_search_sql #102
Conversation
blargh! sorry I must have seen the email for this, but forgot about it. taking a look now |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for this! a few comments, and requests.
also, can you please add a minimal test file for upsert
#' @template key | ||
#' @template args | ||
#' @references \url{http://bit.ly/1G9cnBl} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please add some examples here
#' against. | ||
#' @param force (logical) Set to \code{TRUE} to edit a read-only resource. | ||
#' Default: FALSE | ||
#' @param method (string) Set to \code{insert}, \code{upsert} or \code{update} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you add either here or in @details
or @section
what these different options are for the method
param
#' Default: FALSE | ||
#' @param method (string) Set to \code{insert}, \code{upsert} or \code{update} | ||
#' Default: 'upsert' | ||
#' @param records (list) The data, eg: \code{[{"dob": "2005", "some_stuff": |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the example data you give here is JSON. can you give an example that is R specific, or at least say what is allowed to be passed in. A data.frame? A list? Both? something else?
@@ -13,9 +13,10 @@ | |||
#' } | |||
#' @importFrom httr status_code | |||
|
|||
ds_search_sql <- function(sql, url = get_default_url(), as = 'list', ...) { | |||
ds_search_sql <- function(sql, url = get_default_url(), as = 'list', | |||
config = httr::add_headers(Authorization = get_default_key()), ...) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why not just use key = get_default_key()
? in the function definition, and then add_headers(Authorization = key)
internally in the GET
call
@jklanger any thoughts? |
… the helper function 'tojun' to 'null' instead of NULL
Description
The added ds_upsert function uses the ckan-api-datastore_upsert to execute three different actions: upsert, update and insert. Form and description style of the ds_upsert function are heavily oriented on the ds_create function. To choose which action shall be executed a method parameter was added.
The small change in ds_search_sql simplifies the usage of authorization-keys and per default uses
the key provided by get_default_key. Before the config had to be added manually. With this default a authorization key set during setup will automatically be used for the query.
Example
ds_upsert(resource_id="db-key", records=data.frame(key1, key2, value), force=FALSE, method="upsert")