Add more metadata for pipelines #399

MSattrtand · 2025-07-03T09:29:56Z

Resolves partially kbss-cvut/s-pipes-editor-ui#195
Resolves kbss-cvut/s-pipes-editor-ui#206

blcham

Done just partial review ... please rebase as well

s-pipes-core/src/main/java/cz/cvut/spipes/engine/ExecutionEngineImpl.java

blcham

see my comments

blcham · 2025-07-18T15:37:56Z

s-pipes-core/src/main/java/cz/cvut/spipes/constants/SPIPES.java

    public static final Property has_script = property("has-script");

+    public static final Property has_pipeline_execution_status = property("has-pipeline-execution-status");
+    public static final Property has_executed_function_name = property("has-executed-function-name");


Why not have in addition to this has-executed-function, that would represent IRI of the function. I.e. it should be similar to how SPIPES.has_module_id is added.

Not sure if has-execution-function-name is needed as well, but I guess we can leave it like that if you need it.

It actually returns IRI like http://onto.fel.cvut.cz/ontologies/s-pipes/hello-world-example-0.1/execute-greeting, not just execute-greeting. I think I've named it like that because we already have a field has_pipeline_name, that returns Script IRI, so it follows the same naming logic

I guess we can resolve this one right?

Not quite yet. As far as I understood, it should be IRI in GraphDB too? In this case, it's still literal, so we can't. But I finally understand how to fix it, so I'll soon push it.

Should be done now. All things that should be IRIs are actually IRIs now.

blcham · 2025-07-18T15:44:04Z

s-pipes-core/src/main/java/cz/cvut/spipes/logging/AdvancedLoggingProgressListener.java

            addProperty(pipelineExecution, SPIPES.has_pipeline_execution_finish_date_unix, finishDate.getTime());
            addProperty(pipelineExecution, SPIPES.has_pipeline_execution_duration, computeDuration(startDate, finishDate));
            addProperty(pipelineExecution, SPIPES.has_pipeline_name, pipelineName);
+            addProperty(pipelineExecution, SPIPES.has_pipeline_execution_status, "FINISHED");


We want to represent status using IRI instead of literal! Moreover, we should represented through specific entities which will be connected to ?execution using rdf:type.

I.e. we would have two new constants in Vocabulary.java:
http://onto.fel.cvut.cz/ontologies/dataset-descriptor/failed-pipeline-execution
http://onto.fel.cvut.cz/ontologies/dataset-descriptor/finished-pipeline-execution

And there would be an ?execution and its related triples would be:

?execution a ddo:pipeline-execution, ddo:failed-pipeline-execution

or

?execution a ddo:pipeline-execution, ddo:finished-pipeline-execution

This is done in JOPA using @types, see https://github.com/kbss-cvut/termit/blob/782f2db881d6c882ebdda810207af54f52601805/src/main/java/cz/cvut/kbss/termit/model/UserAccount.java#L183 for inspiration.

Done. I'm not sure that I've added it in the correct way, since s-pipes-model.ttl has Generated by the OWL API in it, so I don't know if it won't be overwritten at some point

-- good catch w.r.t. Generated by the OWL API but it is ok : this is added here only if you are editing this file using Progege tool, which is ok.

-- however you modified s-pipes-model.ttl incorrectly see my comments on that file and try to read this thread again.

s-pipes-core/src/main/java/cz/cvut/spipes/logging/AdvancedLoggingProgressListener.java

blcham · 2025-07-18T15:48:01Z

s-pipes-core/src/main/java/cz/cvut/spipes/logging/AdvancedLoggingProgressListener.java

+
+            String pipelineName = metadataMap.get(SPIPES.has_pipeline_name.toString()).toString();
+            addProperty(pipelineExecution, SPIPES.has_pipeline_name, pipelineName);
+            addProperty(pipelineExecution, SPIPES.has_pipeline_execution_status, "FAILED");


should be IRI represented using @types ... as explained above.

not done as i can still see addProperty(pipelineExecution, SPIPES.has_pipeline_execution_status ...., why should be described elsewhere ...

It's definitely an IRI now. Should be done

blcham · 2025-07-18T15:53:50Z

s-pipes-core/src/main/java/cz/cvut/spipes/logging/AdvancedLoggingProgressListener.java

    }

-    private void persistPipelineExecutionStarted(final EntityManager em, long pipelineExecutionId, Thing pipelineExecution) {
+    private void persistPipelineExecutionStarted(final EntityManager em, long pipelineExecutionId, Thing pipelineExecution, final String functionName, final String scriptPath) {


I would add scriptIRI in addition to scriptPath ... as scriptIRI is typically something that does not change even if scriptPath is changed. We want to log that !

scriptIRI basically represents the identity of the script -- it should ideally not be changed if the script is doing the same task ... it must change if it is doing something else.

It already has Script IRI like http://onto.fel.cvut.cz/ontologies/s-pipes/hello-world-example-0.1 in has_pipeline_name field, or in Script Name columns at the executions page.

I do not understand what do you mean by that ...

Do you mean that there is a triple:
?execution :has-pipeline-name http://onto.fel.cvut.cz/ontologies/s-pipes/hello-world-example-0.1 . ?

If so, we should call it :has-pipeline instead.

or wait ... it is not pipeline

it is :has-script right ?

Yes, we have triples like ?execution :has-pipeline-name http://onto.fel.cvut.cz/ontologies/s-pipes/hello-world-example-0.1

We have has-script property, but it's never used

So should I just rename has-pipeline-name and has-executed-function-name to has-pipeline and has-executed-function respectively?

If it is pointing to IRI than it is not name ... it is the object itself ...

You are however pointing to Script and not the Pipeline ... so you should name it:
has-pipeline-name --> has-script
has-executed-function-name --> has-executed-function

See some insights into terminology here

blcham · 2025-07-19T20:00:19Z

s-pipes-core/src/main/java/cz/cvut/spipes/engine/ExecutionEngineImpl.java

-        fire((l) -> {l.pipelineExecutionFinished(pipelineExecutionId); return null;});
-        return outputContext;
+        String functionName = module.getFunctionName();
+        String scriptPath = module.getScriptPath();


I am also wondering how to align your approach to what was done in https://github.com/kbss-cvut/s-pipes/pull/406/files#diff-2c6ee28836a277018037b96e4cf50d4f76b07f4c6a8f31bbaac8e20e4fc24df9

This requires discussion.

We have to wait for #406 to be resolved.

s-pipes-core/src/main/java/cz/cvut/spipes/logging/AdvancedLoggingProgressListener.java

s-pipes-debug/src/main/java/cz/cvut/spipes/debug/dto/PipelineExecutionDto.java

blcham · 2025-07-23T07:41:55Z

s-pipes-core/src/main/java/cz/cvut/spipes/logging/AdvancedLoggingProgressListener.java

+            try {
+                URI script = new URI(metadataMap.get(SPIPES.has_script.toString()).toString());
+                addProperty(pipelineExecution, SPIPES.has_script, script);
+            } catch (URISyntaxException e) {
+                log.error("Invalid script URI in metadata map: {}", metadataMap.get(SPIPES.has_script.toString()), e);
+                throw new IllegalStateException("Invalid script URI in metadata map", e);
+            }


You are polluting code with try block that basically cannot fail because you are using it only privately:

private static final Map<String, Object> metadataMap = new HashMap<>();

Then you have exactly same code 30 lines after this ... I am surprised that Intellij idea did not give you hint that it is duplicate code ...

Moreover, i suggest to create private method getURIFromMetadataMap() that handles all that. I.e. new code will look like:

addProperty(pipelineExecution, SPIPES.has_script, getURIFromMetadataMap(SPIPES.has_script));

blcham · 2025-07-23T07:42:20Z

s-pipes-core/src/main/java/cz/cvut/spipes/logging/AdvancedLoggingProgressListener.java


-            String pipelineName = metadataMap.get(SPIPES.has_script.toString()).toString();
-            addProperty(pipelineExecution, SPIPES.has_script, pipelineName);
+            try {


see comment above.

blcham · 2025-07-23T07:48:28Z

s-pipes-core/src/main/java/cz/cvut/spipes/logging/AdvancedLoggingProgressListener.java

+                addProperty(moduleExecution, SPIPES.has_script, module.getResource().toString().replaceAll("\\/[^.]*$", ""));
+                if(!metadataMap.containsKey(SPIPES.has_script.toString())){
+                    metadataMap.put(SPIPES.has_script.toString(), module.getResource().toString().replaceAll("\\/[^.]*$", ""));


Suggested change

addProperty(moduleExecution, SPIPES.has_script, module.getResource().toString().replaceAll("\\/[^.]*$", ""));

if(!metadataMap.containsKey(SPIPES.has_script.toString())){

metadataMap.put(SPIPES.has_script.toString(), module.getResource().toString().replaceAll("\\/[^.]*$", ""));

String script = module.getResource().toString().replaceAll("\\/[^.]*$", "");

addProperty(moduleExecution, SPIPES.has_script, script);

if(!metadataMap.containsKey(SPIPES.has_script.toString())){

metadataMap.put(SPIPES.has_script.toString(), script);

duplicate code but i still do not understand why we need to do this ... can you explain me whole this part what is doing, why we need it ?

Done. It saves the script's name as a property to be added to the database, then saves it to metadataMap if it's not there. We need to have it there so we'll have access to it from any function in the logger. E.g. right now I have a code like:
private void persistPipelineExecutionStarted(final EntityManager em, long pipelineExecutionId, Thing pipelineExecution, final String functionName, final String scriptPath)
Where functionName and scriptPath are parameters of the function I've added. To avoid using them as inputs of the function, I should use metadataMap and put these parameters there. It will also allow for easy access to these parameters from any other function if needed.

blcham · 2025-07-23T08:00:52Z

s-pipes-core/src/main/java/cz/cvut/spipes/constants/SPIPES.java

    public static final Property has_output_content = property("has-output-content");
    public static final Property has_script = property("has-script");

+    public static final Property has_pipeline_execution_status = property("has-pipeline-execution-status");


didn't we decide to move it to type of ?pipelineExecution ?

blcham

see my comments

blcham · 2025-07-23T08:04:54Z

s-pipes-model/src/main/resources/s-pipes-model.ttl

+###  http://onto.fel.cvut.cz/ontologies/dataset-descriptor/finished-pipeline-execution
+s-pipes:finished-pipeline-execution rdf:type owl:DatatypeProperty ;
+                     rdfs:domain ddo:module-execution ;
+                     rdfs:range xsd:boolean ;
+                     rdfs:label "has finished successfully" .
+
+###  http://onto.fel.cvut.cz/ontologies/dataset-descriptor/failed-pipeline-execution
+s-pipes:failed-pipeline-execution rdf:type owl:DatatypeProperty ;
+                     rdfs:domain ddo:module-execution ;
+                     rdfs:range xsd:boolean ;
+                     rdfs:label "has finished with error" .


Suggested change

### http://onto.fel.cvut.cz/ontologies/dataset-descriptor/finished-pipeline-execution

s-pipes:finished-pipeline-execution rdf:type owl:DatatypeProperty ;

rdfs:domain ddo:module-execution ;

rdfs:range xsd:boolean ;

rdfs:label "has finished successfully" .

### http://onto.fel.cvut.cz/ontologies/dataset-descriptor/failed-pipeline-execution

s-pipes:failed-pipeline-execution rdf:type owl:DatatypeProperty ;

rdfs:domain ddo:module-execution ;

rdfs:range xsd:boolean ;

rdfs:label "has finished with error" .

### http://onto.fel.cvut.cz/ontologies/dataset-descriptor/finished-pipeline-execution

s-pipes:finished-pipeline-execution rdf:type owl:Class ;

rdfs:subClassOf s-pipes:pipeline-execution ;

rdfs:label "Pipeline execution that finished successfully" .

### http://onto.fel.cvut.cz/ontologies/dataset-descriptor/failed-pipeline-execution

s-pipes:failed-pipeline-execution rdf:type owl:Class ;

rdfs:subClassOf s-pipes:pipeline-execution ;

rdfs:label "Pipeline execution that finished with error .```

i guess we did not understand each other... please read my comments again.

Should be done

blcham · 2025-07-23T08:08:06Z

s-pipes-core/src/main/java/cz/cvut/spipes/logging/AdvancedLoggingProgressListener.java

            addProperty(pipelineExecution, SPIPES.has_pipeline_execution_finish_date_unix, finishDate.getTime());
            addProperty(pipelineExecution, SPIPES.has_pipeline_execution_duration, computeDuration(startDate, finishDate));
            addProperty(pipelineExecution, SPIPES.has_pipeline_name, pipelineName);
+            addProperty(pipelineExecution, SPIPES.has_pipeline_execution_status, "FINISHED");


-- good catch w.r.t. Generated by the OWL API but it is ok : this is added here only if you are editing this file using Progege tool, which is ok.

-- however you modified s-pipes-model.ttl incorrectly see my comments on that file and try to read this thread again.

blcham · 2025-07-23T08:10:19Z

s-pipes-core/src/main/java/cz/cvut/spipes/logging/AdvancedLoggingProgressListener.java

+
+            String pipelineName = metadataMap.get(SPIPES.has_pipeline_name.toString()).toString();
+            addProperty(pipelineExecution, SPIPES.has_pipeline_name, pipelineName);
+            addProperty(pipelineExecution, SPIPES.has_pipeline_execution_status, "FAILED");


not done as i can still see addProperty(pipelineExecution, SPIPES.has_pipeline_execution_status ...., why should be described elsewhere ...

MSattrtand requested a review from blcham July 3, 2025 09:36

blcham requested changes Jul 3, 2025

View reviewed changes

s-pipes-core/src/main/java/cz/cvut/spipes/engine/ExecutionEngineImpl.java Outdated Show resolved Hide resolved

MSattrtand force-pushed the improve-pipeline-metadata branch from 12cec5c to 0f355a6 Compare July 18, 2025 13:12

blcham force-pushed the improve-pipeline-metadata branch from 0f355a6 to cdd3d2d Compare July 18, 2025 15:29

blcham requested changes Jul 19, 2025

View reviewed changes

blcham reviewed Jul 19, 2025

View reviewed changes

blcham reviewed Jul 22, 2025

View reviewed changes

blcham self-requested a review July 23, 2025 07:45

blcham reviewed Jul 23, 2025

View reviewed changes

blcham self-requested a review July 23, 2025 08:11

blcham requested changes Jul 23, 2025

View reviewed changes

MSattrtand force-pushed the improve-pipeline-metadata branch from 3f011cc to adc41d3 Compare October 10, 2025 10:27

Evgenii Grigorev added 16 commits October 17, 2025 14:54

Add more metadata for pipelines

17f8548

Add interface for pipelineExecutionFailed to ProgressListener.java

01070ca

Removed workarounds that are no longer necessary

0958cf1

Changes status from literal to IRI

ca4af92

Removed exessive logging

c50cbac

Renamed parameters to match their usage

603523a

Change the script type to URI from string

b174607

Move extraction of script URI to the separate function

6ebdcd8

Fix incorrect .ttl file

f4abee6

Remove duplicate code

a17921a

Change Strings to URIs

933aa93

Change IRI of pipeline_execution_statu

c018127

Fix metadataMap not being cleared

0dfac0a

Fix execution page not working

3e52659

Update ExecutionEngineImpl to utilise data from inputContext

3d26abe

Fix the test failure

516ae21

MSattrtand force-pushed the improve-pipeline-metadata branch from 0412d55 to 516ae21 Compare October 17, 2025 12:54

Add more metadata for pipelines #399

Are you sure you want to change the base?

Add more metadata for pipelines #399

Uh oh!

Conversation

MSattrtand commented Jul 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

blcham left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

blcham left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MSattrtand Jul 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MSattrtand Jul 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

blcham Jul 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

MSattrtand commented Jul 3, 2025 •

edited

Loading

MSattrtand Jul 21, 2025 •

edited

Loading

MSattrtand Jul 24, 2025 •

edited

Loading

blcham Jul 22, 2025 •

edited

Loading