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

feature: Add Resources section #541

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open

feature: Add Resources section #541

wants to merge 10 commits into from

Conversation

panaaj
Copy link
Member

@panaaj panaaj commented Mar 14, 2019

As per issue #540 the specification could benefit from the addition of a Resources section to describe the way in which to work with these data items.

The initial content outlines the proposed creation, updating and deletion of resources and also proposes some basic filters to use when retrieving resources to limit the number of records returned.

As there is no documented way of supplying parameters to a Delta message I have taken a leap and proposed a params attribute for a get delta message.

I have referenced and aligned with other parts of the specification where possible.
Also tried to align POST and PUT requests with common use patterns.

There probably needs to be some text around security and accessing individual resource records as well.

Some other operational behaviour may also need to be specified e.g. Should the creation, updating and deletion of a resource trigger a delta update message?
I would assume yes but it's probably best that expected behaviour is specified.

@panaaj panaaj changed the title Add Resources section feature: Add Resources section Mar 15, 2019
@panaaj
Copy link
Member Author

panaaj commented Mar 17, 2019

Resources can link / reference to other resources e.g. Route start and end link to waypoint records.
If a route is deleted should the associated a start and end waypoints be deleted as well?

@rob42
Copy link
Contributor

rob42 commented Mar 23, 2019

Resources can link / reference to other resources e.g. Route start and end link to waypoint records.
If a route is deleted should the associated a start and end waypoints be deleted as well?

I think this is implementation specific. Obviously you should not delete a resource that is referenced from elsewhere, but I dont think thats a part of the signalk specification. The spec should say that " deletion is attempted and may fail, eg if the resource is referenced from elsewhere, or permissions etc. See request/response"

Copy link
Contributor

@rob42 rob42 left a comment

Choose a reason for hiding this comment

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

I think the params object is good, but we need to make it a generic part of signalk to use it well.

Also naming needs to be collision-free and suitable for any circumstance, So:

"params": { 
       //one (or more?) of
        "geo": {radius": 10000, //meters
                   "bounds": [138.23, -38.123, 139.76,-37.89], //array[ W-lon, S-lat, E-lon, N-lat ], eg SW to NE rectangle
                   "hash": "ssdfrt" 
                   },
          "limit": 20,
          "fromRecord": 0
}

Need generic params for recordCount in response too. Im thinking along the lines of SQL, and REST results paging...

@panaaj
Copy link
Member Author

panaaj commented Mar 30, 2019

Should then the http parameters format align to make them similar to stream api?
e.g. ?filter={geo: {radius:1000} }

@rob42
Copy link
Contributor

rob42 commented Mar 30, 2019

Is that valid http? Probably geo.radius=1000&geo.etc would be more std

@panaaj
Copy link
Member Author

panaaj commented Apr 1, 2019

Parameters will likely need to encompass:

  • Generic values e.g. limit=xxx, etc
  • Collections like e.g. geo.radius=xxx, etc
    and
  • Resource attributes e.g. note/region, region/geohash, chart/scale, etc

As resources have few common attributes, it would make it easier to identify an attribute used as a parameter if it were:

  1. name-spaced i.e. note.region
    OR
  2. Scoped with attr e.g. attr.region=xxx

So a query for all notes attached to the same region would be something like ../resources/notes?attr.region=123456

And clearly identified in use with others ../resources/notes?attr.region=123456&limit=200&geo.radius=1000

…he `resources` path and identify the four resource types defined in the schema.
@fabdrol
Copy link
Member

fabdrol commented Jun 5, 2019

@panaaj @rob42 what's the status of this, more work required? If so @rob42 can you review and clearly identify what @panaaj should do to finish this PR?

@panaaj
Copy link
Member Author

panaaj commented Jun 22, 2019

Given the intent of this PR was to have resources represented in the spec I have removed the section that talked about the ability to filter returned records as this needs more consideration.

There are clearly implementation considerations as well as specification additions to be made to add entry filtering, delta parameters, etc so rather than hold up this PR I have removed them.

@fabdrol @rob42

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.

3 participants