sckott / cowsay

cowsay w/ more animals, in R
https://sckott.github.io/cowsay/
Other
308 stars 49 forks source link

fillerama api seems to be back up. #80

Closed mnr closed 3 years ago

mnr commented 3 years ago

Description

fillerama API seems to work fine, so I uncommented the related code. As a precaution, I've wrapped the jsonlite call with try() to trap out api errors.

Related Issue

Example

say(what="futurama") # this should work.

sckott commented 3 years ago

Thanks @mnr

@aedobbyn Thoughts on this? I'm thinking maybe we don't rely on this API that could go down again probably pretty easily.

mnr commented 3 years ago

I've been pointing to cowsay in a couple of R weekly sessions. One of my students pointed out the missing "lorem ipsum" - no big deal if this is removed - but just to let you know...

mnr commented 3 years ago

oops - didn't mean to close the pull

aedobbyn commented 3 years ago

thanks @mnr!

I'm thinking maybe we don't rely on this API that could go down again probably pretty easily.

hmm yeah... how often has it gone down? I'm not at all attached to it so my instinct would also be to just remove, but if we want to keep it in maybe we handle errors with a tryCatch or a purrr::possibly?

mnr commented 3 years ago

I put in a simple try(,,) but only to keep it from crashing out - nothing graceful.

Would you like me to add a more interesting handler if the API isn't available? I'd be happy to close this pull, add something more elegant, then re-pull....

aedobbyn commented 3 years ago

Yeah I think something that lets the user know what's going on if the API is down makes more sense. Either that or we take this out entirely. Up to you @sckott

sckott commented 3 years ago

Okay, we can keep it.

However, there needs to be better error handling. e.g,

what <- try(jsonlite::fromJSON(paste0('http://api.chrisvalleskey.com/fillerama/get.php?count=1&format=json&show=', what))$db$quote )
if (inherits(what, "try-error")) stop(what)

haven't tested that, but something like that

If code is left as is right now, then on an error it would just return futurama as the string in the output because what would retain its original value as passed in by the user

mnr commented 3 years ago

I've added filleramaIsBroken to substitute something when the fillerama api is broken. I would LOVE to return something appropriate to "what" but can't seem to figure out how to pass that variable to filleramaIsBroken. Please let me know if I'm missing something obvious.

aedobbyn commented 3 years ago

If I'm understanding what you want to do, you could do something like

if (what %in% c("arresteddevelopment", "doctorwho", "dexter", "futurama", "holygrail", "simpsons", "starwars", "loremipsum")) {
    what_orig <- what

    check4jsonlite()
    what <- tryCatch(
      jsonlite::fromJSON(paste0("http://api.chrisvalleskey.com/fillerama/get.php?count=1&format=json&show=", what))$db$quote,
      error =
        function(e) {
          return(paste0("`", what_orig, "` isn't available right now :("))
        }
    )
  }

so that would return

> say(what = "starwars")

 -------------- 
`starwars` isn't available right now :( 
 --------------
    \
      \
        \
            |\___/|
          ==) ^Y^ (==
            \  ^  /
             )=*=(
            /     \
            |     |
           /| | | |\
           \| | |_|/\
      jgs  //_// ___/
               \_)

Or if you want it to say something besides just what_orig you could make a key-value dataframe for the phrase you want it to say given a what_orig if that makes sense

sckott commented 3 years ago

nice one @aedobbyn , looks good

mnr commented 3 years ago

Thanks @aedobbyn !

I was trying to incorporate a separate function to keep all the awkward error handling elsewhere. But Amanda's solution works great.

aedobbyn commented 3 years ago

cool! you could definitely pull that into a function if you wanted to. up to you!

mnr commented 3 years ago

let me clean up and resubmit...