[Code Review] Fetch JSON & store in Ecto repo

Questions / Help
1

Hey all,

I’ve got a small side project where I made a Vue.js app to search the topics/content of my podcast. It was previously using the Simplecast API directly, but their API responses aren’t the fastest and they’ve had some CORS bugs over the last couple of months. Last night I setup a quick app to get that JSON and store it in an Ecto repo.

It’s a pretty simple example, but I’d love some feedback. What would you change? How would you handle a potential error response from an external API?

Simplecast.ex

3 Likes
2

Cool!

It’s a good start I think. A few things I noticed:

  1. Regarding the error handling, I think it’s fine to just let it crash. What happens now is that it fails silently (IO.puts). When writing a lib you could return a tuple from fetch_episodes, but in this case I would just use get!.

    The request may fail for any number of reasons of course, but, whatever the reason is, the request is retried the next day anyway, and the crash should appear in the log (if im not mistaken). An error reporting service could be used to get notified of the crash.

  2. All three functions end with _episode. This can be an indication that this is an entity by itself that deserves a separate module, for example Simplecast.API.Episode.

  3. find_or_create_episode can be implemented more efficiently using ‘on conflict’.

  4. You could use a transaction to prevent only a part of the episodes being persisted

I implemented the changes here: https://github.com/thomasbrus/dnc-topic-api/compare/6ace5a684f909e268a3302d11a631e13af994d6a...6b704442ffd26a39c07aca7fe75de8a49aedbc09

Hope thats helpful. Cheers!

P.S. Your API key is still visible on Github in the commit history

3 Likes
3

Thanks for the feedback!

Re: the API key, thanks for reminding me about that. It’s read only, but that sill should be taken care of.

– Edit

Removed that key.

3 Likes