18F / micropurchase

18F's micro-purchase threshold experiment management app.
https://micropurchase.18f.gov
Other
68 stars 34 forks source link

API responses are too verbose #741

Open harrisj opened 8 years ago

harrisj commented 8 years ago

Since I am in the process of formally documenting the API responses and types, I've realized we have a bit of redundant and unnecessary information in them. I would like to revise the API responses to not share as much in the next version of the API

  1. The first big thing is that each auction returns a list of all bids (each of which have a bidder object). The api/v0/auctions returns a list of all these auction objects which means it is some pretty verbose JSON as we add more auctions. I would suggest that we limit information on bids/bidders to the api/v1/auctions/:id requests. Should we need to provide a listing of all bids for all auctions, we could potentially add a api/v1/bids request, but I'm not sure people need it.
  2. Looking at the responses for a particular auction, we have the following fields. I have bolded and commented on ones I want to excise:

Auction:

So what? These are a few fields, but I feel like we should share as little about other users as is required for transparency and I don't see how some of these DB modification fields are helpful. So I'd like to cut them and clean up a few other things as well. Note that this doesn't affect the admin API calls which contain everything. Thoughts?

harrisj commented 8 years ago

I want to add that this is not a judgment of the original API which was awesome and created by awesome developers. Just figuring out things I'd like to change for the next version as we grow!

stvnrlly commented 8 years ago

I don't have strong feelings, but want to point to the GitHub API as one that I think is successfully verbose.

adelevie commented 8 years ago

When I took the first pass at version 0 of the API, I was in a hurry and wanted to just get the most data released with the least amount of code.

The initial use case was to allow @vzvenyach to get bidding data on his own, without his needing to ask me to SSH in, convert the Ruby to JSON and paste it into a GitHub Gist using the Gist API (true story, I did this a couple times). I also wanted access to all of the data to be easy. Since the data was small (still sort of is I think), I figured a single endpoint, /auctions, could give everything to API users. No paginating, no constructing new URLs etc.

So the short term goal was just get data out there, make it available, and move on to code other things.

Now that the API has had some time in the real world, we probably know a bit more about use cases. All that said, I think it's good practice to release as much data as we can, as a matter of course. We need not contemplate specific uses for each and every field. But as a principle, if we're storing it, it matters by some measure.

My feelings on those fields aren't strong, and I probably won't miss them. I just want to be sure we don't fall into a trap where any field whose use by the public we can't contemplate needs to be justified.

adelevie commented 8 years ago

Another thought:

If/when we start using AJAXy things on the site, it'll likely become more and more important that there be parity between the API and ActiveRecord.

harrisj commented 8 years ago

Fair enough, I am fine with redundant fields, but I would like to remove the unnecessary created_at/updated_at fields (unless they are useful for cache invalidation I guess).

One other thing: is there a reason we don't have the delivery_deadline in the public auction responses?

adelevie commented 8 years ago

One other thing: is there a reason we don't have the delivery_deadline in the public auction responses?

I don't think so. We should include it.

harrisj commented 8 years ago

Okay, fair enough. These fields can say. I will admit that I am a bit of an API minimalist, mainly because it's often much harder to remove fields than it is to add them later. But let's keep it for now. I would like to do this though:

  1. Let's have the /auctions endpoint not return bids
  2. Add delivery_deadline to the auction JSON

Sound good?

harrisj commented 8 years ago

Two other things of course:

  1. Any changes would be for /v1 of the API
  2. Have we considered adding some metadata like bid_count to the auction serializer (should that go in winning_bid?)

In terms of how to do this, it'd probably mean the following steps:

  1. Move the current serializers to a serializers/v0 directory and make sure that works.
  2. Copy the v0 controllers and serializers (and swagger file) to v1 equivalents (add new routes).
  3. Make changes to the v1 version
  4. Email users that v1 is the new version and v0 will be removed in N months
  5. (passage of time)
  6. Delete the v0 routes and paths