-
Notifications
You must be signed in to change notification settings - Fork 8
759 upgrade edc client to v3 #776
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
base: master
Are you sure you want to change the base?
Conversation
b77be62 to
61b79cc
Compare
|
@bnouwt First comment on changes in the Knowledge Directory. We will not be able to avoid sending the EDC participant ID and protocol URL to other KERs through the Knowledge Directory as a KER needs this information to do a catalog request of another participant and it cannot get this information from anywhere else. So the changes in its API input are necessary I would say |
bnouwt
left a comment
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.
Hi @DaviddeBest-TNO, you did a nice job with this pull request.👍
As usual I have been extra picky for the review and this results in quite a few comments, but most are not really mandatory fixes. One thing I do think requires fixing is the fact that users have to manually build the testkd and testsc docker images for the example to run successfully. We either need to add this to the README.md or make sure the docker images are publicly available (which it should after we release the next version of the KE).
As mentioned before, I do not really like the additional KER properties being distributed via the knowledge-directory. I did try to find ways to not having to (see various comments), so I would like to discuss those with you. If we conclude those are not feasible/possible/desirable, then I will accept these additional fields.😢
So, let's discuss this PR further and get it merged!
Regards, Barry
...connector-api/src/main/java/eu/knowledge/engine/smartconnector/api/SmartConnectorConfig.java
Show resolved
Hide resolved
...in/java/eu/knowledge/engine/smartconnector/runtime/messaging/RemoteKerConnectionManager.java
Show resolved
Hide resolved
.../src/main/java/eu/knowledge/engine/smartconnector/runtime/messaging/RemoteKerConnection.java
Outdated
Show resolved
Hide resolved
.../src/main/java/eu/knowledge/engine/smartconnector/runtime/messaging/RemoteKerConnection.java
Show resolved
Hide resolved
.../src/main/java/eu/knowledge/engine/smartconnector/runtime/messaging/RemoteKerConnection.java
Outdated
Show resolved
Hide resolved
…olicy/contract definition and catalog request
…process to v3 of EDC API
b3c7bc1 to
a1e6602
Compare
bnouwt
left a comment
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.
Gogogo!
No description provided.