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

Move providers to independent node module #522

Closed
nichtich opened this issue Nov 26, 2019 · 24 comments
Closed

Move providers to independent node module #522

nichtich opened this issue Nov 26, 2019 · 24 comments
Labels
cleanup code cleanup, refactoring, testing...
Milestone

Comments

@nichtich
Copy link
Member

For #74 a first part to be published as independent node module (jskos-providers?) is providers. This would allow to unit-test providers and to implement other clients. First use client could be a command line client to look up schemes, concepts, mappings, and mapping recommendations.

Requires #320 and maybe #500.

@nichtich nichtich added the cleanup code cleanup, refactoring, testing... label Nov 26, 2019
@nichtich nichtich added this to the 1.8.0 milestone Nov 26, 2019
@nichtich
Copy link
Member Author

Configuration of providers should be very clear before this step.

  • It looks like provider configuration values "reconcile", "api", and "status" serve the same purpose for different provider types ("ReconciliationApi", "SkosmosApi", "ConceptApi"). Either the three are disjoint and can get the same name to simplify configuration or a registry can have combinations of them but then the provider types don't make sense.

  • Can field "data" be renamed to "concepts" or is it also used to retrieve other entities?

@stefandesu
Copy link
Member

  • It looks like provider configuration values "reconcile", "api", and "status" serve the same purpose for different provider types ("ReconciliationApi", "SkosmosApi", "ConceptApi"). Either the three are disjoint and can get the same name to simplify configuration or a registry can have combinations of them but then the provider types don't make sense.

I think status makes sense because that represents the status endpoint from which the other endpoints are retrieved. But I guess we could also rename it to api or some other uniform name. I agree that there doesn't need to be a differentiation between reconcile and api, and the same thing applies to occurrences as well I would say. We could just call it a uniform name and depending on the provider, it is used differently (for ConceptApi and MappingsApi, it is the status endpoint to retrieve server config, for Reconciliation it is the reconcile URL, ...). What do you say?

  • Can field "data" be renamed to "concepts" or is it also used to retrieve other entities?

It was used to retrieve details for schemes in the past, but looking at the code and the network requests, it is not used for that anymore. The thing is that concepts currently refers to the /voc/concepts endpoint to retrieve all concepts (or all concepts for a certain scheme). Therefore, before renaming anything here, we need to clarify this.

@stefandesu
Copy link
Member

By the way, I would suggest having either separate modules for each provider (including the base provider which they all inherit), or decide on a set of default providers which are bundled as something like jskos-default-providers and offer specific providers (if necessary) in separate modules.

@nichtich
Copy link
Member Author

We could just call it a uniform name and depending on the provider, it is used differently

Then how about endpoint?

By the way, I would suggest having either separate modules for each provider

I'd prefer a bundle to avoid module inflation and to simplify documentation. jskos-providers could include the base provider and the most used providers. Special provider (such as SPARQL endpoint provider) could be added in separate modules.

@stefandesu
Copy link
Member

Then how about endpoint?

👍

I'd prefer a bundle to avoid module inflation and to simplify documentation. jskos-providers could include the base provider and the most used providers. Special provider (such as SPARQL endpoint provider) could be added in separate modules.

Okay, that's fine with me.

@stefandesu
Copy link
Member

From #498 (@nichtich):

To facilitate creation of additional applications such as #497 and to add unit tests (#54), all methods that communicate with API endpoints (JSKOS-Server, Skosmos API...) should be put into a node module (#74, maybe cocoda-sdk). Given a config file with configuration of registries, it should be easy to look concepts and mappings without having to know about API endpoints, e.g.:

const cdk = require('cocoda-sdk')(config)
const schemes = await cdk.getSchemes()
const concept = await cdk.getConcept({uri})
const mappings = await cdk.getMappings({from, toScheme})

@stefandesu
Copy link
Member

It seems like we are going to push this forward because it could be used in other applications as well. So let's try to clarify things about the providers here:

  1. So should we have both jskos-providers (which are only the provider classes) and a cocoda-sdk (which is basically a convenience wrapper around the providers)?

  2. We should take another look at the current API of the providers and the registries in Cocoda and make some adjustments.

  3. I think it would be nice if it was possible to add (external) providers in Cocoda via the config file, but I'm not sure how to achieve that. The providers need to be known at build time and installed and included in the build bundle, so I don't think it would be possible to do it without rebuilding the application. The only thing I can think of is loading it via an external script which would add some more initial loading time and probably increase overall size because that external script would need to include the base provider as well. Any ideas, or is this something we should leave for the future?

@nichtich
Copy link
Member Author

So should we have both jskos-providers (which are only the provider classes) and a cocoda-sdk

jskos-providers should be enough, no?

The providers need to be known at build time

We only have a limited set of providers as referenced via config file field registries[].provider, right? Having support of these hard-coded into Cocoda (via jskos-providers) is ok. The current providers don't add much code, do they?

@stefandesu
Copy link
Member

jskos-providers should be enough, no?

The thing is: I want jskos-providers to only have the provider classes, so basically a way of putting in a registry object and getting back an instantiated provider that can be used to query that registry.

What you want with cocoda-sdk is all the functionality around it, so given a group of registries (as in the Cocoda config file), give me all concept schemes or give me info about that concept (without having to specify which registry can provide information about that concept), etc.

Do you understand what I mean?

We only have a limited set of providers as referenced via config file field registries[].provider, right? Having support of these hard-coded into Cocoda (via jskos-providers) is ok.

I'm not completely sure what your point is, but I would solve this like we did before: Each provider (that is defined in jskos-providers or elsewhere) has a predefined "name" that is used to reference it in the config file field registries[].provider. Of course jskos-providers will be included in Cocoda so any of those providers can be used for the registries. What my point was that if someone wanted to offer their own provider as a package, there would be no easy way to be able to use it with a default Cocoda installation. It would be necessary to fork it and import the package and add a line of code so that Cocoda knows that there are more providers. But maybe it's not even necessary to think about that right now because we don't even have that case at the moment.

The current providers don't add much code, do they?

Can you specify what you mean by that? If we moved the providers out of Cocoda into a jskos-providers package, there would be almost no code needed in Cocoda to be able to use them.

@nichtich
Copy link
Member Author

What my point was that if someone wanted to offer their own provider as a package, there would be no easy way to be able to use it with a default Cocoda installation.

If someone knows how to code a provider, he/she should also be able to extend Cocoda this way. It's ok if Cocoda has a fixed set of supported providers which can be configured.

@stefandesu
Copy link
Member

If someone knows how to code a provider, he/she should also be able to extend Cocoda this way. It's ok if Cocoda has a fixed set of supported providers which can be configured.

Okay. 👌

@stefandesu
Copy link
Member

The thing is: I want jskos-providers to only have the provider classes, so basically a way of putting in a registry object and getting back an instantiated provider that can be used to query that registry.

What you want with cocoda-sdk is all the functionality around it, so given a group of registries (as in the Cocoda config file), give me all concept schemes or give me info about that concept (without having to specify which registry can provide information about that concept), etc.

Do you understand what I mean?

@nichtich You haven't said anything about this anymore. Do you understand my reasoning? What should we do? I could imagine the following three options:

  1. Have a jskos-providers package, but no cocoda-sdk package. Application need to do things like aggregation of results by themselves.

  2. Have separate packages for jskos-providers and cocoda-sdk like I described.

  3. (Probably the most sensible option) Have a single package (whether we name it jskos-providers or cocoda-sdk, I personally like the latter) that offers both "high level" access (i.e. give me all schemes for configured providers) and "low level" access to providers (so direct access to provider classes).

@nichtich
Copy link
Member Author

Thanks for the reasoning 😄 So let it be option 3!

@stefandesu
Copy link
Member

I'm currently thinking about what kind of interface cocoda-sdk should have. My thoughts:

  • The (internal) providers will have methods that correspond to the respective HTTP methods, so getMappings, getMapping, postMapping, putMapping, patchMapping, deleteMapping, ...
  • The external interface will have the exact same methods, but they will have an additional parameter registry (or registries?) where the registries to be queried can be specified.
    • My thought was that if no registry is given, cocoda-sdk will (for GET requests) query all registries that support it and merge the results. Question: How do we deal with pagination in this case?
    • I don't think Cocoda needs to query multiple registries at the same time anywhere except for concept schemes (where there is fairly complicated code that prioritizes and merges the results). It would make things much easier if a registry was required. For concept schemes, we could still add an additional methods getAllSchemes or something.
    • BUT not allowing to query multiple registries would not fulfill your thoughts in one of the comments above where one could just provide a configuration (with one or more registries) and start querying it without considering the individual registries. What do you think? (Although, thinking about it, at least getting concept data via URI would be possible if all schemes were loaded because we can look up which scheme corresponds to the concept URI and query that scheme's registry.)
  • I would also add support for other network requests, like retrieving the configuration via URL (which is what Cocoda is doing), retrieving concept lists, querying build-info.json to see if there's an update, etc. With this, we would basically solve Refactor API requests and background activity #536 at the same time, especially if we offered a way to suspend all repeating (background) requests. (This would of course be optional because other applications might not need it.)

All in all, this is going to be a little more complicated than just moving the providers out of Cocoda. But I think it is a good and important step.

@stefandesu
Copy link
Member

I'm also thinking about whether cocoda-sdk should deal with managing and connecting existing data, e.g. making sure each concept's inScheme field refers to the actual scheme object that was already loaded, and getting new data would update existing objects instead of replacing them, etc. Basically what the current "objects" mixin in Cocoda is doing. But maybe this is way too much and should be a separate module...

@stefandesu
Copy link
Member

I was just thinking: Is cocoda-sdk even the right name, or should it rather be coli-conc-sdk?

@nichtich
Copy link
Member Author

I was just thinking: Is cocoda-sdk even the right name, or should it rather be coli-conc-sdk?

I'd prefer the first name but nevermind

@stefandesu
Copy link
Member

I'd prefer the first name but nevermind

cocoda-sdk it is. 😉

@stefandesu stefandesu modified the milestones: 1.8.0, 1.3.1 May 7, 2020
@stefandesu
Copy link
Member

I think cocoda-sdk is basically ready to be integrated into Cocoda. I'm expecting significant changes though because it will require some refactoring.

@stefandesu
Copy link
Member

The first integration of cocoda-sdk into Cocoda is now on branch "cocoda-sdk". There are a lot more tests and cleanups I have to do though before we can merge this into dev.

@stefandesu
Copy link
Member

I'm going to use this issue to also do some overdue refactoring.

stefandesu added a commit that referenced this issue Jun 19, 2020
@stefandesu
Copy link
Member

Further development will happen in branch test and will (soon) automatically deploy to https://coli-conc.gbv.de/cocoda/test/ for testing.

@stefandesu
Copy link
Member

With the latest commit, I would consider my work on this issue done. @nichtich Let's test a little bit more before closing and merging from test into dev. It would also be good if you could look into #571 again because as I said, I can't reproduce it at all.

@stefandesu
Copy link
Member

Testing is still necessary, but any bugs should be reported as their own issue. I would suggest merging test into dev later today or tomorrow.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cleanup code cleanup, refactoring, testing...
Projects
None yet
Development

No branches or pull requests

2 participants