Implement code header for interactions through use of EDC#723
Implement code header for interactions through use of EDC#723DaviddeBest-TNO merged 26 commits intomasterfrom
Conversation
… with remote runtimes
…a-info & initial verification of authorization token
Callback not received. Not sure why yet.
Note that you need to start the EDC Connectors first manually and afterwards start the rest of the docker containers. Done: - When KER starts, it configures its EDC connector with the standard TKE asset. - When a new RemoteKER is found, we negotiate a contract and start a transfer. Then we receive a authentication token that we use for all communication with that particular KER. Todo: - remove hardcoded otherKER from edc.properties file. - include kb3 in example. - automatically revalidate token when it becomes invalid.
…PI and with simplified connector configuration
| These Knowledge Interactions are first registered at the Smart Connector. | ||
| After they have been registered, they can be executed. | ||
|
|
||
| For more information on the Knowledge Engine, check out the [documentation](../../docs/00_home.md). |
There was a problem hiding this comment.
Link is no longer valid. Replace with: https://docs.knowledge-engine.eu/
| - `connector/configuration/ker-configuration.properties` contains settings for the EDC-IDS Connector | ||
| - `connector/configuration/ker-vault.properties` contains a public key | ||
| - A new directory for this specific KER. Currently, they are named `ker1`, `ker2`, ..., containing: | ||
| - `edc.properties` file containing several properties that are used in the communication between KERs |
There was a problem hiding this comment.
This one is probably no longer necessary?
| For each additional KER with an EDC-IDS Connector, we need the following files in the `examples/edc-example` directory: | ||
| - `connector/configuration/ker-configuration.properties` contains settings for the EDC-IDS Connector | ||
| - `connector/configuration/ker-vault.properties` contains a public key | ||
| - A new directory for this specific KER. Currently, they are named `ker1`, `ker2`, ..., containing: |
There was a problem hiding this comment.
A separate directory is no longer necessary now the edc.properties file is replaced by the default KE configuration that is being modified using environment variables in the docker-compose.yml file.
|
|
||
| The `docker-compose.yml` in `examples/edc-example/` should also be modified to include: | ||
| - An additional KER (currently named `runtime-1`, `runtime-2`, ...) | ||
| - The `build` setting refers to the directory for the new KER. |
There was a problem hiding this comment.
build: can probably be replaced with image:.
| - An additional KER (currently named `runtime-1`, `runtime-2`, ...) | ||
| - The `build` setting refers to the directory for the new KER. | ||
| - The `depends_on` setting refers to the Docker component for the EDC-IDS Connector | ||
| - The `KE_RUNTIME_EXPOSED_URL` is a unique URL for the new KER. |
There was a problem hiding this comment.
Include configuration environment variables that used to be in edc.properties.
|
I think it is not necessary to add documentation about EDC functionality to https://docs.knowledge-engine.eu/, but maybe we can add a short section in the |
There was a problem hiding this comment.
Like I mentioned above, this file can probably be removed.
There was a problem hiding this comment.
I hope this file can be deleted, because we do not want it there.
| 2. In the `knowledge-directory` directory in this project, execute `docker build . -t testkd:1.2.5-SNAPSHOT`. | ||
| 3. In the `smart-connector-rest-dist` directory in this project, execute `docker build . -t testsc:1.2.5-SNAPSHOT`. |
There was a problem hiding this comment.
Check if we still need this.
There was a problem hiding this comment.
Also check the version numbers, because these are probably no longer correct.
There was a problem hiding this comment.
I updated the version numbers. For now we still need this, but I agree that a more elegant solution is better in the future.
| # This is the knowledge directory, facilitating discovery between different | ||
| # runtimes. It exposes its service over port 8282. | ||
| knowledge-directory: | ||
| image: testkd:1.3.2-SNAPSHOT |
There was a problem hiding this comment.
This version number should be 1.3.3-SNAPSHOT I think.
| @@ -0,0 +1,2 @@ | |||
| FROM docker.io/library/testsc:1.3.2-SNAPSHOT | |||
There was a problem hiding this comment.
This version number should be 1.3.3-SNAPSHOT, I think.
| @@ -0,0 +1,2 @@ | |||
| FROM docker.io/library/testsc:1.3.2-SNAPSHOT | |||
There was a problem hiding this comment.
This version number should be 1.3.3-SNAPSHOT, I think.
| @@ -0,0 +1,2 @@ | |||
| FROM docker.io/library/testsc:1.3.2-SNAPSHOT | |||
There was a problem hiding this comment.
This version number should be 1.3.3-SNAPSHOT, I think.
|
|
||
| # authCode token expiration in seconds | ||
| #edc.transfer.proxy.token.validity.seconds=999999 | ||
| edc.transfer.proxy.token.validity.seconds=120 |
There was a problem hiding this comment.
Because this token validity is set to 120 seconds, the edc-example stops working (i.e. one of the KBs stops returning data) after 2 minutes. Maybe change this to the same value as the other connectors?
| "tke-dataplane", | ||
| "", | ||
| "", | ||
| "http://tke.tno.nl", |
There was a problem hiding this comment.
Maybe change this to https://www.knowledge-engine.eu/?
| // these are all static for every connector | ||
| public final static String DATA_PLANE_ID = "tke-dataplane"; | ||
| public final static String ASSET_NAME = "TNO Knowledge Engine Runtime"; | ||
| public final static String ASSET_URL = "https://www.knowledge-engine.eu/"; |
There was a problem hiding this comment.
What is the difference between ASSET_URL and the assetUrl in the previous file? Probably nothing.
There was a problem hiding this comment.
Something to clean up later
|
I have also tested the behavior of the empty configuration options when using EDC and this does not give any problems when |
| kd.url = http://localhost:8080 | ||
| sc.validate.outgoing.bindings.wrt.incoming.bindings = true | ||
| ke.runtime.hostname = localhost | ||
| ke.reasoner.level = 2 No newline at end of file | ||
| ke.reasoner.level = 2 | ||
| ke.runtime.use.edc = false |
There was a problem hiding this comment.
These red ^M character mean that this file uses windows line endings, which is wrong. I've already create issue #724, because some other files in the repo also use this, but maybe for this file we can already change it to using LF (line feed).
| edcConnectorUrl: | ||
| type: string | ||
| format: uri |
There was a problem hiding this comment.
I wondered what happened with this property when we did not configure it via the configuration options and it looks like this:
[
{
"id": "http://localhost:8081",
"exposedUrl": "http://localhost:8081",
"protocolVersion": "1.0.0",
"lastRenew": "2025-08-29T11:16:39.984815+02:00",
"edcConnectorUrl": null
}
]
This is OK I guess, but I do find it a pity that it is necessary. We could think about whether we could leave it out if not applicable. This might also break backwards compatibility of the Knowledge Directory and KE Runtimes, so we should at least mention this in the next release of the KE.
There was a problem hiding this comment.
Now edcConnectorUrl is only set if not null
| KnowledgeEngineRuntimeConnectionDetails ker = new KnowledgeEngineRuntimeConnectionDetails(); | ||
| ker.setExposedUrl(myExposedUrl); | ||
| ker.setProtocolVersion(PROTOCOL_VERSION); | ||
| ker.setEdcConnectorUrl(myEdcConnectorUrl); //TODO: make optional, or can just be null? |
There was a problem hiding this comment.
This TODO from (probably) @Sophietje is spot on 😉
There was a problem hiding this comment.
Now made optional!
There was a problem hiding this comment.
I am a bit scared for this class, since its error handling has been improved the last year or so. I assume those improvements were correctly merge, but it would be a pity if we lost them.
I've quickly double checked and compared the master branch (and its recent history) of this file and compared it with this version and it seems to be fine.
There was a problem hiding this comment.
Are there certain things that you would like me to check? We can also discuss in person
There was a problem hiding this comment.
Maybe double check if the error handling cases that currently happens in the master branch are also handled in this branch? If you also think they are similar (like I suggest above) then it is fine by me.👍
There was a problem hiding this comment.
What is this empty class doing here? The naming Tken... is also quite strange. If I remove it locally everything still builds, but I did not test whether the EDC example also still works.
There was a problem hiding this comment.
I truly have no idea what this is. Will remove it!
bnouwt
left a comment
There was a problem hiding this comment.
Hi @DaviddeBest-TNO, please don't be alarmed by the many comments😉. When reviewing we should just mention as many things as possible and afterwards determine which ones are actually worthwhile/necessary to address.
I have tested the edc-example and it also works for me locally, so that is good news!
I think some of the suggestions are worthwhile and easy to address, but we can discuss the others. Maybe go over the comments yourself and fix those that are easy and you agree with. After that we can discuss the remaining ones. Thanks for the effort👍
Personally I don't think the main README of this repo would be the right place for that. It's already quite long and it's not necessary information for most (?) users. Perhaps we can find a better spot? Maybe a README elsewhere in the repo (near the example?). Otherwise the docs.knowledge-engine.eu could be a good spot if we add something like a header saying 'experimental' |
I made an attempt to add a small text on current developments to the README, also referring to the README of the EDC example for more info. This something we think is suitable for now? And then change to something more elaborate later in the project? |
For me this is fine👍 What do you think @Sophietje? |
bnouwt
left a comment
There was a problem hiding this comment.
Thanks for processing the comments @DaviddeBest-TNO . You can merge if you think it is finished 👍
Users can toggle the EDC functionality through setting the KE_RUNTIME_USE_EDC environment variables.
This sets an authorization code header of messages between KERs by using features from EDC.
A working examples is in the
examplesfolder.