Implementation of extension #1
Conversation
| {"required": ["template:new_field"]}, | ||
| {"required": ["template:xyz"]}, | ||
| {"required": ["template:another_one"]} | ||
| {"required": ["marble:host_node"]}, |
There was a problem hiding this comment.
Does this mean that either host_node OR is_local is required? Shouldn't both be required at the same time?
There was a problem hiding this comment.
No, it doesn't mean that. Both are required and extension in the present form will complain if either one is not present.
These tests are run on GitHub too. All tests are passing. The tests are done against the sample implementations in the "examples" folder. This is the typical |
|
Just found this extension: https://github.com/stac-extensions/storage It seems like representing a self-hosted data would simply be marked by Interest could be raised here (stac-extensions/storage#22) to indicate that more details about the location for this " edit: I raised the issue in the STAC Community Meetup of 2024-02-26. The |
|
I don't think I'm qualified enough to review this intelligently. |
|
@dchandan @mishaschwartz |
Thanks @fmigneault for raising that. If we can get the storage extension to support our use-case then I have no issue using it |
Yes I agree, especially with Ouranosinc/Magpie#589 |
|
I also like the idea of the storage extension. But, my concern here would be how soon they can update the extension to have the flexibility that we need? There is a bit of a time constraint, so one option would be to just go with the marble extension in the meantime. One thing worth considering is future expansion potential with a marble extension. We could use the extension to, in the future, include all sorts of other marble network related information. |
|
@dchandan I agree as long a |
|
@fmigneault @mishaschwartz PR is good to go now? |
json-schema/schema.json
Outdated
| "marble:host_node", | ||
| "marble:is_local" |
There was a problem hiding this comment.
These fields are nested under the "type": {"const": "Collection"} definition.
This whole block should be removed, or nested into a not disallowing "type": "collection" explicitly.
There was a problem hiding this comment.
Good catch! done.
There was a problem hiding this comment.
Just to be sure, is the oneOf to allOf change caused by a lib complaining about a single-item in oneOf?
There was a problem hiding this comment.
No, I just changed it. Schema works in the present form with either one. Since it was just one case right now, I felt allOf was more readable.
There was a problem hiding this comment.
If there's only one item then you could just remove the allOf/oneOf key entirely, right? Wouldn't that be the most readable since then its clear that there's only one option?
There was a problem hiding this comment.
I think it needs to be under one of the keys (again, based on studying other implementations, I'm not a schema-guy myself).
There was a problem hiding this comment.
I think most implementations leave them with the oneOf no matter how many entries are defined to make them extensible more easily without major refactors. They are also more similar between extensions, making them easier to read when switching between them.
A version 1.0.0 implementation with simply two core fields:
marble:host_nodeandmarble:is_local.