-
Notifications
You must be signed in to change notification settings - Fork 1
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
Issue with different APIs requiring different scheme URIs #43
Comments
|
Providers should try best as can to find out which URI ( Solution 5 also seems reasonable but requires other changes in BARTOC. |
Okay. I will implement solution 4 (or 2, or a combination of those two) for now, but long-term we would want to use 3 or 5. |
- this._api.schemes will not contain an array of schemes anymore. - ConceptApi: getSchemes will always perform request, then filter by the list of schemes in this._jskos.schemes if necessary.
While I was fixing this issue I realized that even in ConceptApi, solution 2 was actually the intended behavior, so that's what I have now implemented. 👍 |
This could possible belong to https://github.com/gbv/bartoc.org rather than here.
Related to #41.
There is an issue with determining a scheme's API by using BARTOC's
API
field. In particular, the issue is that certain APIs require a specific scheme URI to be used for requests.For example: If you currently run
node examples/bartoc.js
on the dev branch, it will say that they are 0 top concepts for BK. The API for BK is DANTE, and DANTE is not aware of bartoc.org URIs, it seems. Thus, when calling the/voc/top
endpoint with the bartoc.org URI (which is in the mainuri
field and therefore used for the call), it returns 0 results (without an error message).Previously, this wasn't an issue in Cocoda because Cocoda didn't use registries based on the scheme's
API
field (initialized byregistryForScheme
). But this is an issue in BARTOC, as you can see for BK: https://bartoc.org/en/node/18785#content And since we are planning to use this for Cocoda as well (see gbv/cocoda#670), this is going to become an issue there as well.There are multiple potential solutions:
Force all APIs to support BARTOC URIs. This shouldn't be an issue for DANTE in particular, but I don't think it's realistic to enforce this for every single API.
Determining the scheme's "main URI" by calling the API's endpoint for schemes and saving the
uri
that it returns. In theory, this is what we have done before, see ConceptApi's and SkosmosApi's_getSchemeUri
method. However, when initializing a registry from theAPI
field, we don't ever call the API's scheme endpoint and therefore never know which is the main URI. In theory, we can adjust the existing methods to ignore the field_api.schemes
that is currently used and call the API's scheme endpoint regardless. Not sure if this is the best or most efficient solution though. (See more on this below.)In the data of the
API
field, add an additional (optional) fielduri
that is the main URI that this particular API is using for this scheme. This might be the easiest solution, even if it requires some editorial work.Use all available identifiers in each call where the scheme URI is needed. This would work for ConceptApi since it should support multiple URIs separated by
|
(this works in DANTE as well). This would be the easiest to implement and will work as long as this issue only exists for ConceptApi.One additional thing: SkosmosApi seems to already use solution 2 and ignores the
_api.schemes
field as far as I can tell. I'm actually not sure if this is intended behavior, but it definitely fixes this issue. I think the reason we're using the_api.schemes
field in ConceptApi is that we wanted to be able to restrict a certain registry to a subset of its schemes. However, instead of directly returning_api.schemes
if it is a list of schemes, we could still call the scheme endpoint and simply filter the results with those schemes that are in the_api.schemes
list. If we do this, solution 2 would probably be the best solution for this issue, in particular because the basis for this is already implemented. (I need to see if this is actually possible.)@nichtich
The text was updated successfully, but these errors were encountered: