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

What to do with NA input to gh()? #21

Closed
jennybc opened this issue Nov 18, 2015 · 14 comments
Closed

What to do with NA input to gh()? #21

jennybc opened this issue Nov 18, 2015 · 14 comments
Milestone

Comments

@jennybc
Copy link
Member

jennybc commented Nov 18, 2015

If ... in gh(endpoint, ...) contains an NA, gh() errors with this:

 Error in if (endpoint2 != endpoint) { : 
  missing value where TRUE/FALSE needed 

Obviously you'd never do that intentionally. I'm using gh() over rather long lists of (alleged) GitHub users, repos, teams, etc. and sometimes this just happens. I wonder if there's a nicer way to handle?

I'm working on another PR re: errors and will also try to propose something for this.

@gaborcsardi
Copy link
Member

Thanks! I am not surprised. As you no, there are no tests. You are my tester. Sorry about it.

So, yes, I am very much looking forward to it.

I'll also try to find some time to set up some test infrastructure, but I don't think that it will happen within 1-2 weeks.

@jennybc
Copy link
Member Author

jennybc commented Nov 18, 2015

Your idea re: building a table of endpoints is starting to make more sense to me now! It would be useful for tests (and other purposes too, I think). That Python (?) wrapper I linked to on some other thread basically does that to generate their tests. I would be willing to lend a hand ... but not until my teaching ends (first week of Dec).

@gaborcsardi
Copy link
Member

Sounds good to me!

Back then I did some quick searches to find some (even partial) declarative description of the GitHub API, but failed. There are some "API generators", that take such a description, and then generate an API client. (Not in R, obviously.)

As you say, we could use such a description to generate test cases.

Even just creating such a description in a language agnostic way has some value. I'll take another look in a minute.

@jennybc
Copy link
Member Author

jennybc commented Nov 18, 2015

http://apievangelist.com/2014/06/05/what-are-the-incentives-for-creating-machine-readable-api-definitions/

although that mostly sounds like something the API creator is supposed to do?

@jennybc
Copy link
Member Author

jennybc commented Nov 18, 2015

Or it could be a really tedious web-scraping task. 😕

@gaborcsardi
Copy link
Member

That page has some good links. E.g. this looks good:
https://github.com/apiaryio/api-blueprint/

And there a lot of others, e.g. here: https://news.ycombinator.com/item?id=8912897

It is not easy to search for a GitHub api spec, because all these tools are actually on GitHub.

You know what, I'll just ask GitHub support....

@gaborcsardi
Copy link
Member

Question sent.

Btw., you said the Python client has this already? Let me take a look.

@gaborcsardi
Copy link
Member

Well, they have it in "Python code", so that is not really ideal....

@gaborcsardi gaborcsardi mentioned this issue Nov 18, 2015
@gaborcsardi
Copy link
Member

GH support replied, they don't have a machine-readable api spec, but they want to create one. No ETA, though.

@gaborcsardi
Copy link
Member

I mentioned this already for another issue, but there is a good list of endpoints here: https://github.com/mikedeboer/node-github/blob/master/lib/routes.json

@gaborcsardi
Copy link
Member

To return back to the original issue, I think NAs should generate errors, no? What else could we do in this case?

@jennybc
Copy link
Member Author

jennybc commented Nov 16, 2016

I agree.

I now call gh() almost exclusively in a purrr::safely(gh)() form. To cope with all manner of errors. That probably didn't exist or I didn't know about it when I opened this issue.

In fact, I wonder if if makes sense to build anything like that into gh()?

@gaborcsardi
Copy link
Member

I don't think I would run in safely be default. Users can do that if they want, I would just return good errors, i.e. using https://github.com/ropenscilabs/fauxpas or https://github.com/sckott/httpcode.

@jennybc
Copy link
Member Author

jennybc commented Nov 16, 2016

No, not by default.

I should clarify that I'm usually using safe_gh() inside purrr::map2() or purrr::pmap(). So I am noticing a very standard set of gymnastic moves that I do before (creating safe_gh()) and after (looking at the errors and, God willing, extracting the "good results").

@gaborcsardi gaborcsardi added this to the 1.1.0 milestone Jan 7, 2020
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

2 participants