-
Notifications
You must be signed in to change notification settings - Fork 82
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
PR for feature #58 (access private datasets and be able to upsert and full replace datasets) #64
PR for feature #58 (access private datasets and be able to upsert and full replace datasets) #64
Conversation
NOTE the new branch. Original work I did was on a branch that was not based on `dev` - Added `email` and `password` parameters to `returnData.R` and its dependent methods such as those in `errorHandling.R` and `metadata.R` - Added tests in `test-readPrivateDataset.R` and ran `document()` - Was able to run `build()` successfully
- created `writeData.R` which defines `write.socrata()` - Added tests in `test-writeData` and ran `document()` - Was able to run `build()` successfully
I am seeing some tests fail locally but I think it is code not related to my changes in the |
} | ||
|
||
|
||
#' @description Method for updating Socrata datasets |
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.
title is missing
as pointed out by @dmpe at Chicago#64 (comment) thanks!
How may I transmit the username/password used for the tests that require a Socrata login? I have been able to successfully test on my own travis-ci.org repo using environment variables. Thanks! |
http://kieranhealy.org/blog/archives/2015/10/16/using-containerized-travis-ci-to-check-r-in-rmarkdown-files/ may help or again what i said in #58 |
You can't use containerized Travis unless sudo isn't required.. I also think it seems an admin of the repo is best off just adding the environment variables from the Travis interface Mark Silverberg
|
oh, i forgot, sorry. ---------- Původní zpráva ---------- Od: Mark Silverberg [email protected] Komu: Chicago/RSocrata [email protected] Datum: 2. 11. 2015 16:21:35 Předmět: Re: [RSocrata] PR for feature #58 (access private datasets and be " Mark Silverberg (m) 512 826 7004
— Reply to this email directly or view it on GitHub "= |
cc @tomschenkjr |
Still reviewing this pull request. Some issues are showing-up in the process. Here are some quick notes so far:
There are some other issues, but am looking at these two first. |
@tomschenkjr: I am seeing those errors as well but I am not confident they are part of this PR. I am getting those errors on the Thanks, Mark |
Yeah, it's unrelated to the write functions you added so as an FYI at this point. Once I clear up those other issues I'll be looking at these new functions. Sent by Outlookhttp://taps.io/outlookmobile for Android On Mon, Nov 16, 2015 at 4:54 AM -0800, "Mark Silverberg" <[email protected]mailto:[email protected]> wrote: @tomschenkjrhttps://github.com/tomschenkjr: I am seeing those errors as well but I am not confident they are part of this PR. I am getting those errors on the dev branch (which doesn't have this PR merged in) Thanks, Mark Reply to this email directly or view it on GitHubhttps://github.com//pull/64#issuecomment-157020153. This e-mail, and any attachments thereto, is intended only for use by the addressee(s) named herein and may contain legally privileged and/or confidential information. If you are not the intended recipient of this e-mail (or the person responsible for delivering this document to the intended recipient), you are hereby notified that any dissemination, distribution, printing or copying of this e-mail, and any attachment thereto, is strictly prohibited. If you have received this e-mail in error, please respond to the individual sending the message, and permanently delete the original and any copy of any e-mail and printout thereof. |
Any issue now can be profiled using https://github.com/rstudio/profvis.
For the second issue e.g. like this :
Yes, there are 2 ways: Either you use execute another query on the desired dataset with =Count(*) (getQueryRowCount does that) or you fetch that information from the socrata's undocumented |
@dmpe - i would suggest using the |
Thanks, one issue solved. Some code changes will be needed. Assumption must be made that if from me then at earliest in Christmas break. With the second issue, @marks can also try help us. (Not so helpful and not so nicely summarized and I am sorry for it.) Do you know why do some datasets (and btw. some not; see above) have an "invalid" json file, which may cause our issues. (well, I guess these were attempts to fix them: #19 #3 )
Cannot test it now, but as i remember it was failing at least some time ago. |
@dmpe - could you help me understand this issue by sharing a list of datasets that are giving invalid JSON? I am very confident all JSON that we produce and put out fits the standard but it might not be exactly what |
Yeeh, code from #3 is blocked so i cannot test it anymore.
|
After spending huge amount of time on it, I start to think the error is comming somewhere from httr or curl package.
But the same code for
is not an issue. WTF is it (with my PC) ? |
@dmpe @tomschenkjr is there anything I can do to help keep this moving forward? Appreciate all your hard work! |
I think we have a bit of scope creep in this PR, but I'm going to continue to evaluate the parts of it to clean-up the errors. Hope to do it this week. Tom Schenk Jr. Chief Data Officer Department of Innovation and Technology City of Chicago (312) 744-2770 data.cityofchicago.org From: Mark Silverberg [email protected] @dmpehttps://github.com/dmpe @tomschenkjrhttps://github.com/tomschenkjr is there anything I can do to help keep this moving forward? [https://avatars3.githubusercontent.com/u/1799661?v=3&s=400]https://github.com/dmpe dmpe (John Malc) · GitHubhttps://github.com/dmpe Appreciate all your hard work! Reply to this email directly or view it on GitHubhttps://github.com//pull/64#issuecomment-166970369. This e-mail, and any attachments thereto, is intended only for use by the addressee(s) named herein and may contain legally privileged and/or confidential information. If you are not the intended recipient of this e-mail (or the person responsible for delivering this document to the intended recipient), you are hereby notified that any dissemination, distribution, printing or copying of this e-mail, and any attachment thereto, is strictly prohibited. If you have received this e-mail in error, please respond to the individual sending the message, and permanently delete the original and any copy of any e-mail and printout thereof. |
…ow splitted. update description file & squash all the commits
Please see issue (feature request) #58; This is the re-do of PR #63 since it was not correctly based on the
dev
branch