Add upper node limit to subtree service

[chinchliff]

Once we have the number of descendant tips stored in the graph (#63), we should implement a size limit for the subtree query.

[jimallman]

What happens if this limit is exceeded? Will we return a depth-limited tree, or an error/exception message?

[jimallman]

One more question: Is this limit a fixed number, or one provided by the caller?

[jar398]

I would say both. If the limit provided by the caller exceeds the limit allowed by the service, probably this should be an error (400 response).

On Mon, Jun 9, 2014 at 10:41 AM, Jim Allman notifications@github.com wrote:

One more question: Is this limit a fixed number, or one provided by the caller?

— Reply to this email directly or view it on GitHub https://github.com/OpenTreeOfLife/treemachine/issues/81#issuecomment-45497869 .

[chinchliff]

Currently, using the neo4j plugins (i.e. POST), we can return a verbose error wrapped inside JSON. The convention I have been using is to return this:

{ "error": "[error text goes here]" }

...at a minimum whenever an error is encountered. Other elements can be added to the JSON to provide additional info, though in most cases I haven't done that. I can do something else if you like. Once we migrate to GET methods, I suppose Jonathan will have more ideas.

[josephwb]

This is part of the v2 service tree_of_life/subtree. I currently have it set to 25,000 tips (so, not depth), but we may want to test this some more to see what is optimal (testing was mostly against a local instance). As @chinchliff suggested, I handle things this way:

# Metazoa: too big to export
curl -X POST http://devapi.opentreeoflife.org/treemachine/ext/tree_of_life/graphdb/subtree -H "content-type:application/json" -d '{"ott_id":691846}'

{
  "error" : "Requested tree is larger than currently allowed by this service (25000 tips). For larger trees, download the tree directly."
}

A few notes: First, this service no longer supports arguson format; @jimallman should keep using v1 of the service if this format is still required. Second, the depth feature. @chinchliff and I don't think people will want to use this. Besides, the newick version of the depth-limited tree is currently buggy (i.e. the returned tree string is not newick-compliant).

[chinchliff]

So, with the upper node limit in place, I think this issue is resolved. (we can open another one later if we need to revisit the depth limit).

[jar398]

I don't approve of discontinuing arguson in the v2 API because we might want to retire the v1 API before the browser app is upgraded to understand whatever the non-arguson format is (if there is one). Unless you have in mind some other v2 way to get arguson?

On Fri, Sep 12, 2014 at 9:50 PM, Joseph W. Brown notifications@github.com wrote:

This is part of the v2 service tree_of_life/subtree. I currently have it set to 25,000 tips (so, not depth), but we may want to test this some more to see what is optimal (testing was mostly against a local instance). As @chinchliff https://github.com/chinchliff suggested, I handle things this way:

Metazoa: too big to export

curl -X POST http://devapi.opentreeoflife.org/treemachine/ext/tree_of_life/graphdb/subtree -H "content-type:application/json" -d '{"ott_id":691846}'

{ "error" : "Requested tree is larger than currently allowed by this service (25000 tips). For larger trees, download the tree directly."}

A few notes: First, this service no longer supports arguson format; @jimallman https://github.com/jimallman should keep using v1 of the service if this format is still required. Second, the depth feature. @chinchliff https://github.com/chinchliff and I don't think people will want to use this. Besides, the newick version of the depth-limited tree is currently buggy (i.e. the returned tree string is not newick-compliant).

— Reply to this email directly or view it on GitHub https://github.com/OpenTreeOfLife/treemachine/issues/81#issuecomment-55478372 .

[josephwb]

@jimallman agreed to this. But I can add a separate service for him. I think we want it separate, as it was a pain to hide the (unused) depth and format arguments from the newick-flavoured subtree method.

[chinchliff]

I thought the whole point of "versioned" services was that they were public and expected to be stable, whereas the non-versioned ones are not. I see no reason to add argus on to the public (i.e. Documented) v2 services since it is deprecated (unstable) and only Jim should be using it. If we decide to retire the v1 services before the webapp is upgraded, it is trivial (one copy/paste operation) to move the arguson method into a different plugin so Jim can continue using it. That is the contingency plan if that scenario occurs. It could also be done now, but we had other priorities in mind this week, so that specific action (copying the arguson method) did not seem critical.

On Saturday, September 13, 2014, Jonathan A Rees notifications@github.com wrote:

I don't approve of discontinuing arguson in the v2 API because we might want to retire the v1 API before the browser app is upgraded to understand whatever the non-arguson format is (if there is one). Unless you have in mind some other v2 way to get arguson?

On Fri, Sep 12, 2014 at 9:50 PM, Joseph W. Brown <notifications@github.com javascript:_e(%7B%7D,'cvml','notifications@github.com');> wrote:

This is part of the v2 service tree_of_life/subtree. I currently have it set to 25,000 tips (so, not depth), but we may want to test this some more to see what is optimal (testing was mostly against a local instance). As @chinchliff https://github.com/chinchliff suggested, I handle things this way:

Metazoa: too big to export

curl -X POST http://devapi.opentreeoflife.org/treemachine/ext/tree_of_life/graphdb/subtree -H "content-type:application/json" -d '{"ott_id":691846}'

{ "error" : "Requested tree is larger than currently allowed by this service (25000 tips). For larger trees, download the tree directly."}

A few notes: First, this service no longer supports arguson format; @jimallman https://github.com/jimallman should keep using v1 of the service if this format is still required. Second, the depth feature. @chinchliff https://github.com/chinchliff and I don't think people will want to use this. Besides, the newick version of the depth-limited tree is currently buggy (i.e. the returned tree string is not newick-compliant).

— Reply to this email directly or view it on GitHub < https://github.com/OpenTreeOfLife/treemachine/issues/81#issuecomment-55478372>

.

— Reply to this email directly or view it on GitHub https://github.com/OpenTreeOfLife/treemachine/issues/81#issuecomment-55494064 .