reTHINK-project / dev-service-framework

DEPRECATED! Service framework libs have been moved to dev-runtime-core repo
Apache License 2.0
1 stars 2 forks source link

Discovery: add global discovery features #74

Closed pchainho closed 7 years ago

pchainho commented 7 years ago

Pending on this one.

rjflp commented 7 years ago

Is this the same as discussed where: https://github.com/reTHINK-project/dev-registry-global/issues/11 ?

pchainho commented 7 years ago

yes, but to be supported by the discovery lib.

rjflp commented 7 years ago

OK. So it is not related to that issue https://github.com/reTHINK-project/dev-registry-global/issues/11.

It has only to do with integrating, into the runtime discovery lib, the search by GUID. A connector for the global registry already exists in Vertex. So this is a question of interacting with it. Right?

pchainho commented 7 years ago

As far as I understand, since Global Registry does not support GUID discovery by IDP Identifiers, we have to use the discovery service

rjflp commented 7 years ago

@ingofriese Is there a simply way for the runtime to query the discovery service? Is there a connector for a message node?

rjflp commented 7 years ago

@pchainho Should we implement search by GUID using the Global Registry or not?

pchainho commented 7 years ago

Yes, I think it is better.

sbecot commented 7 years ago

The search by GUID can be implemented with a simple http GET and the global registry is public. I don't see any added value in going through the msg node though a connector. Concerning the discovery service, it's basically the same, and more, implementing a connector is adding comlpexity, and oblige to maintain more code. To finally do an http GET. This way of doing tightly couple the componants. You can of course implement such connectors, but as a application developper I would use directly the APIs.

pchainho commented 7 years ago

The runtime is agnostic of protocols or external APIs.

Currently it should only depend on the Catalogue well-known URIs

@rjflp any progress on this?

sbecot commented 7 years ago

I am sorry but this is not true. On the contrary the runtime seems to be linked to the apis of the external components through their APIs, meaning that you need to develop and maintain many connectors. The runtime should be as thin as possible. Implementing a new protocol does not mean that you are agnostic, it means that you use a proprietary one.

pchainho commented 7 years ago

could you be more clear, perhaps with some examples?

I suspect there is some miss-understanding here

jmcrom commented 7 years ago

jst a basic fact: most of the web today is not rethink-compliant, not even rethink-aware

to bring them to rethink we have to shows s that the cost of integration is minimal whilst the benefits are huge

sbecot commented 7 years ago

Just to be more clear, at the begining I was expecting the message node, and the runtime, to be as thin as possible. This means that the msg bus would route messages dedicated to hyperty signaling, even for catalogue and domain registry if you considere msg node as a gate keeper. I see now in your implementation that all is routed in the msg bus, even support services, which means overhead, complexity in the runtime, and coupling.

pchainho commented 7 years ago

@jmcrom I completely agree with you but I think discovery service and global discovery are rethink backend support services

@sbecot the message node is light. Runtime is not so light but that was expected since it is where we have the "business logic" running at the edge. We knew that from the very beginning. Currently we are using the msg node as "gatekeeper" of backend services because that 's the only place where we have a policy engine. Otherwise the runtime could directly interact with these backend services with protostubs. That's also a possibility for the discovery service. It should be straightforward. Another option is to have the discovery service directly connected with the messaging framework as discussed in Lisbon. I think @sdruesedow also has already something implemented for that .

BernardoMG commented 7 years ago

@ingofriese @pchainho

I'm starting looking at this and from the comments above i guess we didn't reach a conclusion related on how to issue queries from the runtime to the discovery service. So, how should the queries be issued? Is there a connector, or the REST endpoints should be used to make HTTP calls, and therefore get the GUID?

Use case: Translate a user url or email to obtain the associated GUID

rjflp commented 7 years ago

Current status:

-Waiting for "java loader" being written to complete integration with Discovery Service -Have completed integration with Global Registry. Still testing before release.

rjflp commented 7 years ago

Apparently we went the wrong way on this one! We implemented the "client side" on the discovery lib (runtime) that enabled the runtime to communicate with the Global Registry via Message Node. We get the replies back, but have no way to verify the signature.

We should have used the Graph Connector, that already does most of this (if not all). @fbeierle is having a look at how to best integrate the Runtime Discovery Lib with the Graph Connector, already present in the runtime.

fbeierle commented 7 years ago

The Graph Connector is a runtime core component that already interacts with the Global Registry. Similar to the Domain Registry, a connector in the Message Node (https://github.com/reTHINK-project/dev-msg-node-vertx/tree/master/src/js/connector) handles the call to the Global Registry REST interface. You can find the code for querying the Global Registry here, including the checks of the signature: https://github.com/reTHINK-project/dev-runtime-core/blob/develop-graphConnector/src/graphconnector/GraphConnector.js#L411

As for integration, I think we could: (a) duplicate code and have the Discovery Lib do the same (b) have the Discovery Lib use the Graph Connector to interact with the Global Registry (c) have the Graph Connector use the Discovery Lib to interact with the Global Registry

I think (c) makes the most sense. The Discovery Lib would then bundle all registry interactions. Please note that for the Graph Connector, we also need to send CREATE messages to the Global Registry: https://github.com/reTHINK-project/dev-runtime-core/blob/develop-graphConnector/src/graphconnector/GraphConnector.js#L288

BernardoMG commented 7 years ago

I agree with @fbeierle. Since we have implemented the exactly same query that Graph Connector does to get GUIDs ("READ") from the Global Registry; however, we didn't verify the signatures.

Using (c), it'll only be necessary to add one more query in the discovery lib that sends CREATE messages to Global Registry.

pchainho commented 7 years ago

This is ok if the discovery Lib is in the runtime core but I believe this was already done: do you confirm @BernardoMG ?

rjflp commented 7 years ago

@fbeierle @pchainho I would say that I prefer option b). The discovery lib would only encapsulate the methods for "discovering" hyperties: Discovery Service, Global Registry and Domain Registry. Eventually, it could also have a method for querying the Graph Connector?

The Graph connector provides more complex functionality (such as creating and inserting GUIDs into the Global Registry) which are not necessary related to searching for other users.

The advantage of keeping the Discovery Lib, would be to encapsulate all the searches:

rjflp commented 7 years ago

@BernardoMG Noticed that we currently have a duplication of code to create and send and receive messages to/from the Message Node. We have it at least in the Discovery Lib and in the Graph Connector. Regardless of how we go forward with the integration, it would be wise to refactor the code to use the same methods for contacting the Message Node.

Is there more code in the runtime to contact the Message Node?

pchainho commented 7 years ago

What kind of duplication? In general, the exchange of messages between the runtime and back-end services are performed disregardless of the Message Node. For example, the discovery lib just has to use the domain registry address domain://registry.<domain> to exchange messages with it.

Anyway, I agree there are some components that should be refactored. At this point I'm not going to name them yet ;-)

rjflp commented 7 years ago

Currently going with @fbeierle option b) to factor all the Discovery Service, Registry interactions into the Discovery Lib.

For communication with the Global Registry, we use the GraphConnector.

Registry.js is being refactored to use Discovery Lib.

This seams to be the way to "save" the most on code duplication.

BernardoMG commented 7 years ago

Discovery Lib is now integrated in runtime-core (branch discovery-integration), supporting several discoveries methods.

In total, it supports nine different discovery methods in respect to hyperties and dataObjects.

It used the branches:

dev-runtime-core: discovery-integration dev-hyperty-toolkit: addressbook-app dev-registry-domain: develop dev-msg-node-vertx: develop dev-service-framework: 74-add-global-discovery-features dev-runtime-browser: develop-GraphConnector-Test

Discovery methods still exists in the service framework to maintain compatibility with the existing hyperties. But they are now deprecated. A deprecated tag was added to these methods.

pchainho commented 7 years ago

I don't find the listener for "/discovery" and corresponding changes in the Service Framework discovery lib. Pls also don't forget to specify the messages used as well as the unit tests

BernardoMG commented 7 years ago

Still working on the exchange of messages between Core Discovery (runtime-core) and Discovery Lib (service-framework).

BernardoMG commented 7 years ago

Messaging between the Core Discovery and Discovery Lib implemented according to this.

You may try it using the following branches:

dev-runtime-core: discovery-integration dev-hyperty-toolkit: addressbook-app dev-registry-domain: develop dev-msg-node-vertx: connector-release-0.8.0 dev-service-framework: 74-add-global-discovery-features dev-runtime-browser: develop-GraphConnector-Test

After this PR is merged into the develop branch in runtime-core, we'll add this implementation to a more recent branch and add unit tests for this component.

pchainho commented 7 years ago

@BernardoMG Graph Connector was merged with Runtime Core develop branch

BernardoMG commented 7 years ago

Discovery methods that could receive the email are now updated in order to use the new query per user email supported by the Domain Registry.

BernardoMG commented 7 years ago

Global discovery features are now merged and working for browser and nodejs.

You may try it using the following branches:

dev-runtime-core: develop dev-hyperty-toolkit: develop dev-registry-domain: develop dev-msg-node-vertx: develop

The testbed will be updated in the next days.

We'll update the test of Discovery in service-framework and add new tests for CoreDiscovery in runtime-core.

This issue will be closed when the tests are updated.

BernardoMG commented 7 years ago

All tests have been updated. This issue is complete.