-
Notifications
You must be signed in to change notification settings - Fork 37
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
Change of inclusion status for coordinate-related properties #203
Change of inclusion status for coordinate-related properties #203
Conversation
…s and assemblies SHOULD NOT appear in the response unless explicitly requested.
While I am in full support for the overall changes; after making these changes we are not so helpful on the underlying intent with what properties get included or not. The reader is likely to end up with the feeling that one needs to carefully consider each individual In reality, our rules are very straightforward:
I think the right place to say this is early in section 4.1, with maybe a callback from 4.2 to say that the same thing applies for single entry endpoints. Or possibly a new subsection. I started to formulate a suggestion, but do not have time to finish it now. (I'll get back to it eventually if no one else makes an attempt.) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks fine to me, thanks @merkys.
I am still not sure about nsites
though (see my comment).
@@ -1692,7 +1692,7 @@ nsites | |||
- **Type**: integer | |||
- **Requirements/Conventions**: | |||
|
|||
- **Response**: REQUIRED in the response unless explicitly excluded. | |||
- **Response**: SHOULD NOT be present in the response unless explicitly included. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Again, for nsites
this seems to intuitively clash with our MUST
for queries (line below).
However, I am empathetic with your situation at COD for how you calculate this number - but that's an implementation issue. For the spec., this seems to be a bit counter-intuitive for me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, right, @merkys: how have you implemented filtering on nsites
? Do we have to remove that from being mandatory as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Answered below.
Just so the above comment isn't lost (since it is inline in a change): Can databases, such as COD, for which it isn't cheap to produce If, no, then we need to remove its mandatory support in filters. This is urgent to fix before the v1.0 release. |
You are right, we cannot allow queries on
I have just now noticed that the requirement level for this is |
Opened PR #205 to relieve the requirement for querying on |
Changed the inclusion status from
REQUIRED in the response
toSHOULD NOT be present unless explicitly included
for the followingstructures
properties:@rartino suggested having a general notice about the exclusion of all but essential properties as a general mechanism. However, I failed to locate a proper place in text (as well in hierarchy of descriptions) to do so. Maybe just changing the inclusion status is enough for time being?
Fixes #184