Generate alternatives#101
Conversation
|
@eladrion Here is the PR for restape. It seems that I don't have any rights to assign you as a reviewer, so I just tagged you. |
There was a problem hiding this comment.
@aaronstrachardt: Is there a reason why you did not use the class TaxonomyElem for this purpose? It is not yet used but could fit here.
There was a problem hiding this comment.
@aaronstrachardt The SnakeYAML approach does work in principle and it was applied for some time in APE. But perhaps the specific CWL parser as used in APE could make things easier. I'm not completely sure but perhaps you could reuse the implementation from APE.
There was a problem hiding this comment.
@eladrion You're right that sharing one CWL parser would be cleaner and it wouldn't add a new dependency.
I checked APE's CWLParser class. The problem is that it only parses a single CommandLineTool, so I cannot reuse it as it is. Reusing it would be possible, but only with an extension to handle workflows.
I think I will stick with the current implementation and add the unified parser as future work. Does that sound okay to you?
There was a problem hiding this comment.
This is okay for now and I can test it as is.
There was a problem hiding this comment.
@aaronstrachardt Fitting approach and would probably work for a prototype. But we should look whether this could be stored in the database instead in a separate table. Nothing you should think about for your plans. Just an Issue I have to add after merging.
|
@aaronstrachardt: your branch is out-of-date with the Restape |
Lombok annotation processing failed on JDK 21+ causing compilation errors. Replaced @Getter, @Setter, @DaTa, @slf4j, @NoArgsConstructor, @AllArgsConstructor, and @requiredargsconstructor with explicit constructors, getters, setters, and SLF4J logger declarations. # Conflicts: # src/main/java/nl/esciencecenter/controller/dto/WorkflowsZip.java
Adds GraphNode, GraphEdge, ApeTaxTuple, and ParseResponse DTOs used by the /alternatives/parse endpoint. All fields have explicit getters for Jackson serialization.
Loads the EDAM OWL ontology asynchronously at startup and builds an inverted URI-to-label map for O(1) lookup. Falls back to the short-form URI fragment while loading. Used to resolve format URIs in CWL inputs/outputs to human-readable EDAM class names.
Parses a CWL v1.2 Workflow document using snakeyaml and produces a graph-optimised ParseResponse: tool nodes, input/output data nodes, directed data-flow edges, and EDAM I/O tuples for APE synthesis constraints. EDAM format URIs are resolved via an injected label resolver function.
Accepts a CWL v1.2 workflow as multipart/form-data, delegates to CwlParser with EDAM label resolution, and returns a ParseResponse with nodes, edges, and I/O EDAM tuples for the frontend graph view.
Reverts a051be8. Lombok 1.18.30 works correctly with Java 17; the annotation-processing issues only affect JDK 21+. Project is now pinned to java.version=17 via spring-boot-starter-parent.
Remove EDAMTaxonomyService (OWL API, async loading, AtomicReference fallback). Bundle pre-built edam_labels.json (3471 entries, 227 KB) as classpath resource. EdamLabels loads it synchronously at startup via Jackson — no network access, no startup delay, no fallback labels.
Replace bundled edam_labels.json with live OWL fetch via EdamLabels. ensureLoaded() blocks synchronously on first call (double-checked lock), subsequent calls return immediately. Add GET /alternatives/warm so the frontend can trigger loading on page mount before the first upload.
EDAM loading is triggered implicitly by the first /parse call. A dedicated warm endpoint adds no value without a frontend consumer.
…le consistency
…rnativesControllerTest
EdamLabels now reads the pre-generated edam_labels.json classpath resource instead of fetching and indexing the EDAM OWL ontology at runtime. This makes label resolution O(1), removes the runtime network dependency, and matches the documented design. The required resource was previously untracked.
aaronstrachardt
force-pushed
the
generate-alternatives
branch
from
a02a615 to
53640f4
Compare
|
Hi @eladrion, I have rebased my branch against upstream/main. Everything should be good now, please let me know if you see any further issues :) |
|
Hi @aaronstrachardt, rebasing resolved the sync issue. Thanks. Apart from that, I left you some comments above which still seem to be correctly referring to the files. |
…a as suggested in the review
…e.java and ParseResponse.java and replace GraphNode's string type with a enum, as suggested in review.
|
Hi @eladrion, I implemented the suggested changes
|
|
Hi @aaronstrachardt, the docker image generation will not work here as the PR source is a non-group repository (and private). I created a branch |
|
I see, your repo is public. But still, your PR is better located to the feature branch. |
eladrion
changed the base branch from
main
to
feat_workflow_alternative_generation
|
Nevermind, could do it myself |
|
I will merge to the feature branch for now |
eladrion
merged commit 7b883d0
into
Workflomics:feat_workflow_alternative_generation
2 participants
Pull Request Overview
This PR adds the backend logic for the "generate alternatives" prototype component. It basically introduces a new
/alternatives/parseendpoint, aCWLParserclass, and some helper logic to process the workflows.Related Issue
No related issue yet.
Changes Introduced
snakeyamldependency for lightweight CWL parsing.CWLParserclass to extract tools and connections from the CWL file.POST /alternatives/parsefor the file uploads.edam_labels.jsonto map URIs to readable labels.How Has This Been Tested?
The code has been tested with the added unit tests (e.g.,
CwlParserTest,EdamLabelsTest). I also tested the whole flow manually on my local machine by connecting it to the corresponding frontend branch.Checklist