Merge Rest api with knowledge gaps into master

[JackJackie]

No description provided.

[bnouwt]

Hey @JackJackie, I finished the first review. What do you think?

[JackJackie]

I'll continue with the REST API changes and will produce a new pull request after that

[bnouwt]

I think you can just keep working in this pull request.

[JackJackie]

Hi @bnouwt : I updated the smart-connector-rest-server /ask route to transfer the knowledge gaps that were found as a result of the request. See the ProactiveApiServiceImpl at line

knowledge-engine/smart-connector-rest-server/src/main/java/eu/knowledge/engine/rest/api/impl/ProactiveApiServiceImpl.java

Line 52 in 99ede29

@Path("/ask")

I kept the old /ask route for convenience and checking...it can be deleted once we are happy with the new /ask route.

knowledge-engine/smart-connector-rest-server/src/main/java/eu/knowledge/engine/rest/api/impl/ProactiveApiServiceImpl.java

Line 187 in 99ede29

@Path("/askold")

It works well on my machine (well-known statement of every developer!). Can you check whether it works for you as well?

Thx, Jack

[bnouwt]

Hey @JackJackie, looks good 👍

If we make sure that the TriplePatterns are represented correctly and we have a unit test (either via the Java API (easy) or via the REST API (difficult)) and all the tests are still succeeding, we are ready to go!

[JackJackie]

Hi @bnouwt,

Before diving into building the optional flag, I ran the unit tests of the smart connector.

There were 60 runs without failures and errors. 3 unit tests were skipped, but I saw that they were @disabled, so this makes sense.

[bnouwt]

I've merged the master branch into this branch and resolved the conflicts. This was not trivial, because the master also contained changes to the *KnowledgeInteraction classes. I am currently running a maven build and hopefully this succeeds and we can let GitHub check if this branch is ready to be merged.

[bnouwt]

Unfortunately, the build did not yet succeed. The DistributedMessageDispatcherTest#testRemoteMessageExchange test somehow fails, but it might be a fluke. For now I've committed the changes and hopefully they build on github.

[bnouwt]

Hi @JackJackie, as mentioned above the build failed due to the REST API Knowledge Gap Unit Tests failing and I found that this is caused by the fact that the default MatchStrategy is not sufficient to find Knowledge Gaps. Unfortunately, it is not possible yet to change the default MatchStrategy from the REST API.

I see a few options to fix this:

• replace the smart connector REST API reasonerEnabled configuration with a matchStrategy configuration.
• use conditional configuration like if knowledgeGapsEnabled, then MatchStrategy.SUPREME to automatically upgrade the MatchStrategy if Knowledge Gaps are requested.
• add a REST API Knowledge Interaction configuration for matchStrategy.

The conditional configuration could make configuring the Knowledge Engine very complex, so I am not sure I would go for that one.

This reason why this problem did not pop-up before is because I merged the master branch into this branch including the new graph pattern matching algorithm with configuring match strategies at the Knowledge Interaction level.

[JackJackie]

@bnouwt : I discovered a small bug in the ProactiveApiServiceImpl around the knowledgeGaps being null.....I need to fix that before we can continue

[JackJackie]

@bnouwt : below here it says...."Changes requested...1 change requested"...I tried to look which change request that is, but couldn't find out. Do you know?

Don't ask stupid questions

[JackJackie]

@bnouwt , @Sophietje , I'm merging the RESTapi with knowledge gaps enabled branch into main....so you can continue with other features based on this new version....hopefully nothing (much) breaks :-)