Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

API v2 #17

Merged
merged 59 commits into from
Apr 24, 2016
Merged

API v2 #17

merged 59 commits into from
Apr 24, 2016

Conversation

kostko
Copy link
Member

@kostko kostko commented Mar 12, 2016

See #1268

)

# Register the viewset with the API router.
api.router.register(r'registry/%s' % model._meta.object_name.lower(), viewset)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we have all top-level registry endpoints under registry/ or should they be at root directly?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would keep it at top level.

@kostko
Copy link
Member Author

kostko commented Mar 12, 2016

Currently, the API does not yet support filtering. For an example output, see this query.

@@ -12,4 +14,6 @@
# Frontend components auto-discovery
components.pool.discover_components()

urlpatterns = components.pool.get_urls()
urlpatterns = components.pool.get_urls() + [
urls.url(r'^api/v2/', urls.include(api.router.urls)),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would move this to nodewatcher/urls.py, so that it is not part of frontend.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, but the whole api is part of the frontend? Why should it be separate?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because to me it is not part of frontend. :-) API can be used by mobile apps, for example. While our frontend app is our "web frontend" in fact.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So to me frontend is this particular Django component system we have and display stuff.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But why is then api under nodewatcher.core.frontend.api and not nodewatcher.core.api?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True. Move it. :-)

@mitar
Copy link
Member

mitar commented Mar 12, 2016

So some features we had in Tastypie and I think we should keep them, so we should check:

  • previous URL if limit=10, offset=5 should exist
  • adding cors headers:
    • response['Access-Control-Allow-Origin'] = '*'
    • response['Access-Control-Allow-Headers'] = 'Content-Type'
    • response['Access-Control-Allow-Methods'] = 'GET, HEAD, OPTIONS'
    • response['Access-Control-Max-Age'] = 60 * 60 # seconds, 1 hour
  • use ujson, and optimization for gis data which provides __json__ to directly provide how it should be serialized
  • limit=0 should give all results (which in fact mean, should limit results to some internal maximum per page (5000?), while default page limit should be something smaller (100?))
  • display correct schema view for (so human/computer readable)
    • click on "options" should have some meaningful description
    • click on get JSON should work (currently you get Registration point 'node.format' does not exist!)
    • click on get API should work (currently you get Registration point 'node.format' does not exist!)
  • prevent access to sensitive data, both for display and filtering
  • allow access to sensitive data based on guardian permissions
  • support global_filter concept (so that you can filter over some concept of all fields at the same time, so I would make it so that based on which fields you are selecting, you can filter over their values, conceptually, whatever is the JSON output, you can filter over that string of the output)
  • have both total count and non-filtered count be in the output (at least when using global filter, but in theory we could use datatables to filter per column as well)
  • I see that in listing view there is no URI to the detail view for each result, Tasypie had that, and we are using it in datastream (which is not part of this), but we might want to keep it? on the other hand, this would be extra data which is often not needed
  • fields=config__core.project should return a primary key for project
  • fields=config__core.project__project should return a whole project subdocument
  • fields=config__core.project__project.name should return a subdocument with only name
  • better names and not Node Registry Root List and Node Registry Root Instance
  • better breadcrumb, I would suggest that it is Root / Node List / <to string of node>
  • integrate the API explorer with the nodewatcher
  • _metadata and _type probably should not be included by default either, no?
  • an extra format which outputs the output (but input stays the same) into NetJSON format
  • support for filtering
  • support for sorting
  • show id fields as @id in documents
  • use __ instead of |
  • optimize speed of conversion between data and JSON
  • create tests against API, especially for all more complicated query types, so that we can detect issues when we will later on try to optimize things
  • add all JSON-LD meta fields, but not necessary all referenced secondary endpoints (vocabulary)
  • filter of all nodes on rectangle gis area (will be used for maps)
  • support complex filters (with and, or, and not)

@mitar mitar assigned kostko and unassigned mitar Mar 12, 2016
@mitar
Copy link
Member

mitar commented Mar 12, 2016

But it looks awesome!

@mitar
Copy link
Member

mitar commented Mar 13, 2016

Maybe we can for now just prevent access to sensitive data and later on open it. Because we do not provide OAuth access to API anyway. Or do we?

@mitar
Copy link
Member

mitar commented Mar 13, 2016

Benchmarking serialization of a GIS field (core.location contains one geolocation, a point) using ab tool (-n 10). API response contains 1074 results, each containing one geolocation. All queries were the same, including all having extra GeoJSON field.

Old Tastypie v1 API with UJSON and our optimization:

Document Path:          /api/v1/node/?format=json&limit=0&fields=location__
Document Length:        276817 bytes

Concurrency Level:      1
Time taken for tests:   17.086 seconds
Complete requests:      10
Failed requests:        0
Total transferred:      2771370 bytes
HTML transferred:       2768170 bytes
Requests per second:    0.59 [#/sec] (mean)
Time per request:       1708.604 [ms] (mean)
Time per request:       1708.604 [ms] (mean, across all concurrent requests)
Transfer rate:          158.40 [Kbytes/sec] received

Connection Times (ms)
              min  mean[+/-sd] median   max
Connect:        0    0   0.0      0       0
Processing:  1443 1708 225.9   1682    2260
Waiting:     1416 1692 229.6   1652    2251
Total:       1443 1709 225.9   1682    2260

Percentage of the requests served within a certain time (ms)
  50%   1682
  66%   1695
  75%   1738
  80%   1831
  90%   2260
  95%   2260
  98%   2260
  99%   2260
 100%   2260 (longest request)

v2 API default JSON:

Document Path:          /api/v2/node/?format=json&limit=0&fields=config|core.location
Document Length:        293849 bytes

Concurrency Level:      1
Time taken for tests:   84.605 seconds
Complete requests:      10
Failed requests:        0
Total transferred:      2940230 bytes
HTML transferred:       2938490 bytes
Requests per second:    0.12 [#/sec] (mean)
Time per request:       8460.512 [ms] (mean)
Time per request:       8460.512 [ms] (mean, across all concurrent requests)
Transfer rate:          33.94 [Kbytes/sec] received

Connection Times (ms)
              min  mean[+/-sd] median   max
Connect:        0    0   0.0      0       0
Processing:  7593 8460 817.1   8352    9959
Waiting:     7581 8442 819.8   8332    9950
Total:       7593 8460 817.1   8352    9959

Percentage of the requests served within a certain time (ms)
  50%   8352
  66%   8419
  75%   8421
  80%   9830
  90%   9959
  95%   9959
  98%   9959
  99%   9959
 100%   9959 (longest request)

v2 API UJSON 1.35:

Document Path:          /api/v2/node/?format=json&limit=0&fields=config|core.location
Document Length:        293849 bytes

Concurrency Level:      1
Time taken for tests:   86.984 seconds
Complete requests:      10
Failed requests:        0
Total transferred:      2940230 bytes
HTML transferred:       2938490 bytes
Requests per second:    0.11 [#/sec] (mean)
Time per request:       8698.397 [ms] (mean)
Time per request:       8698.397 [ms] (mean, across all concurrent requests)
Transfer rate:          33.01 [Kbytes/sec] received

Connection Times (ms)
              min  mean[+/-sd] median   max
Connect:        0    0   0.1      0       1
Processing:  8075 8698 894.2   8389   10975
Waiting:     8045 8681 895.7   8377   10964
Total:       8075 8698 894.2   8390   10975

Percentage of the requests served within a certain time (ms)
  50%   8390
  66%   8747
  75%   8878
  80%   9272
  90%  10975
  95%  10975
  98%  10975
  99%  10975
 100%  10975 (longest request)

With our optimization of using __json__:

Document Path:          /api/v2/node/?format=json&limit=0&fields=config|core.location
Document Length:        293845 bytes

Concurrency Level:      1
Time taken for tests:   78.629 seconds
Complete requests:      10
Failed requests:        0
Total transferred:      2940190 bytes
HTML transferred:       2938450 bytes
Requests per second:    0.13 [#/sec] (mean)
Time per request:       7862.865 [ms] (mean)
Time per request:       7862.865 [ms] (mean, across all concurrent requests)
Transfer rate:          36.52 [Kbytes/sec] received

Connection Times (ms)
              min  mean[+/-sd] median   max
Connect:        0    0   0.0      0       0
Processing:  7241 7863 536.0   7903    8812
Waiting:     7225 7845 538.8   7870    8797
Total:       7241 7863 536.0   7903    8812

Percentage of the requests served within a certain time (ms)
  50%   7903
  66%   8021
  75%   8295
  80%   8444
  90%   8812
  95%   8812
  98%   8812
  99%   8812
 100%   8812 (longest request)

I had to refactor some code because otherwise I was getting an
`AppRegistryNotReady` exception. It seems Django is much more sensitive
now about when things can load.
@mitar
Copy link
Member

mitar commented Apr 21, 2016

What I am saying is that "global filter" is to me some extension point different modules can register themselves in. And can say, "if user is interested (fields) into this registry item, this is how you can global search on it."

@kostko
Copy link
Member Author

kostko commented Apr 21, 2016

What I am saying is that "global filter" is to me some extension point different modules can register themselves in. And can say, "if user is interested (fields) into this registry item, this is how you can global search on it."

So you are saying that as soon as a specific registry item is projected, some fields (declared by the registry item developer) would become part of the "global filter"? So for example, projecting anything under core.project would automatically add config:core.project__project__name to the list of global filters (for example)?

How could client know which of fields have a reasonable text search option?

Well, the client is already allowed to filter by arbitrary fields, including using text expressions. For example, the client may do config:core.project__project__name__icontains=foo.

@mitar
Copy link
Member

mitar commented Apr 21, 2016

So for example, projecting anything under core.project would automatically add config:core.project__project__name to the list of global filters (for example)?

Maybe projecting config:core.project__project__name would add config:core.project__project__name if it is specified as searchable.

@mitar
Copy link
Member

mitar commented Apr 21, 2016

Well, the client is already allowed to filter by arbitrary fields, including using text expressions.

Yes, but the question is how it knows which of those queries can it do? I am not talking here about a person, but client program. So user can reason about this. But how can a "global filter" field know which queries to construct in the API request when user (human) types something in?

@kostko
Copy link
Member Author

kostko commented Apr 21, 2016

But how can a "global filter" field know which queries to construct in the API request when user (human) types something in?

The developer of the client-side application will decide which filters to use? The same as it will decide which columns to show or which filters to apply.

@mitar
Copy link
Member

mitar commented Apr 21, 2016

And what about when user can select which columns to display through UI interface? And datatables display rows. But there is still one field to search over globally.

@mitar
Copy link
Member

mitar commented Apr 21, 2016

But yes, we can do then something like this:

  • you create normal filters
  • documents returned from those filters are those which count towards "total count"
  • you can additionally add second-layer of filters, which are ored together, those count towards "filtered count"

And it will be developers or UIs responsibility to generate this second-layer of filters somehow.

@mitar
Copy link
Member

mitar commented Apr 21, 2016

In general, is there a way to create and and or between filters? Because I was thinking of using that in my UI, but now I do not even believe this is possible through the query language exposed?

@kostko
Copy link
Member Author

kostko commented Apr 21, 2016

The registry supports such queries, but they are currently not exposed via the API. If you think of a nice way to pass such query expressions, we can support it.

@kostko
Copy link
Member Author

kostko commented Apr 21, 2016

We now have support for complex filter expressions using filters.

@mitar
Copy link
Member

mitar commented Apr 21, 2016

Super! I updated the list above. I think only GIS query is now pending. Everything else has been done!

if instance._meta.pk.name in data:
del data[instance._meta.pk.name]
id_field = instance._meta.pk.name
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, if we expose primary key as @id, how can you then know over which field you can filter and sort? Because you probably still have to write uuid to filter, and not @id?

@kostko kostko merged commit 7980373 into development Apr 24, 2016
@kostko kostko deleted the api-v2 branch April 24, 2016 17:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants