[preview] dataset api

[rclark]

• adds work-in-progress dataset api functions to the Mapbox client
• decodes the requesting user's name from the provided access token
• adds tests to fully cover branching in make_service.js
• .gitignore coverage outputs

Additional scopes are needed to pass tests on travis. fixes #7

cc @tmcw @willwhite

[willwhite]

Any benefit to including code versus just parsing using a jwt library?

[rclark]

When I looked for modules I wasn't aware that browserify does some polyfilling, so I was trying to find something that was node + browser. The client side libs for this I found are way overkill for what ends up being incredibly straightforward code. I see no reason to bring in a dependency to do this.

[tmcw]

Looks pretty good: general question: is it a covered usecase for one user to pull/push from another user's datasets? Right now this assumes that token=user, but will that assumption continue to be true?

[tmcw]

could this be puts to match the plural of deletes?

[rclark]

Yes, I'll work this in. I want to make sure that the names of the variables here match the properties of the response object returned from the API.

[rclark]

About other people's datasets: I don't believe that it is currently possible, but it is something that we want to make possible. Datasets' ids make this a little bit clumsy. URLs are like:

GET /datasets/v1/:owner/:datasetid/features?access_token=:token

In the context of requesting your own dataset, you configure the SDK with your access token, and so you only need to provide the :datasetid in order to find your features. In a sense, though, you can see that the dataset's id is not just :datasetid, really it is :owner/:datasetid.

Where a dataset id is required to make a request, a method in this lib looks like

datasets.readFeature = function(featureid, datasetid, callback) {
  // GET /datasets/v1/:user-from-token/:datasetid/feature/:featureid?access_token=:token
};

Even though it is honest, I kinda don't want to change this to

datasets.readFeature = function(featureid, owner, datasetid, callback) {};

... because much of the time the owner will be you and feel redundant. What if instead, add logic to each function that takes a dataset id:

var owner;
var id;
if (datasetid.split('/').length > 1) {
  owner = datasetid.split('/')[0];
  id = datasetid.split('/')[1];
} else {
  owner = this.user; // was determined via access token
  id = datasetid;
}

[willwhite]

Feel free to defer this to another conversation, but I'm curious if you have thought about documenting return types/properties with documentationjs. By using placeholders in this "example", we miss an opertunity to communicate specifics about types or return formats - like the fact that the created property is a ISO formatted string.

Ideally:

• return types would be thoroughly documented
• examples would be pure examples and show real looking values - not place holders

[tmcw]

Upstream in JSDoc, there's no support for documenting parts of return values like you can document parts of params. See previous discussion: documentationjs/documentation#127

[rclark]

I'm 100% with you on the ideal case here. But @returns just isn't the right place to put this. What you really want to do is document the @param {Function} callback, which is a part of JSDoc that seems kinda clumsy and isn't supported in documentationjs.

I do think this is something we ought to elevate to the shortlist in documentationjs: documentationjs/documentation#9

[tmcw]

@rclark do you mean the @callback tag?

[rclark]

Yep @tmcw that's what I was getting at.

[willwhite]

Should we do this type of validation here or let it happen on the server? I could see us making a backward compatible change that removes this requirement.

[rclark]

I'm happy with this for now. Aside from the cross-owner dataset access thing which I'd like to punt on until we're more clear on how we'll implement it, I think that I hit all the issues that you guys spotted. Good to merge @tmcw @willwhite?

[tmcw]

👍 yep!

[willwhite]

step on it 👍