Pipeline Runner Implementation#111
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR implements a comprehensive pipeline runner system with significant architectural changes and updates to core components.
- Adds a new
DockerPipelineRunnerthat orchestrates pipeline execution using Zenoh communication - Restructures pipeline models by moving
PipelineNodefromcore/pipeline.rstomodel/pipeline.rs - Introduces pipeline result tracking with status management and failure logging
- Updates API naming conventions across the codebase (e.g.,
next→process_packet,SpecURI→NodeURI)
Reviewed Changes
Copilot reviewed 36 out of 37 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/pipeline_runner.rs | New integration test for pipeline execution workflow |
| tests/fixture/mod.rs | Added pipeline test fixtures and data structures |
| src/core/pipeline_runner.rs | New pipeline runner implementation with async task management |
| src/uniffi/model/pipeline.rs | Enhanced pipeline models with new status types and result structures |
| src/core/model/pipeline.rs | Moved and expanded pipeline node definitions |
| src/core/operator.rs | Updated operator trait method names and added serialization |
| tests/extra/data/input_txt/* | Test data files for pipeline integration tests |
| .github/workflows/tests.yaml | Updated Rust toolchain version |
Comments suppressed due to low confidence (1)
tests/fixture/mod.rs:1
- Malformed text with broken backticks and typos. Should be 'based on the
pipeline_jobandpipeline_spec'
#![expect(
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| )]))?; | ||
| let namespace_lookup = test_dirs.namespace_lookup(); | ||
|
|
||
| // Create and agent and start it (temporary for now, will be merge later) |
There was a problem hiding this comment.
Grammatical error: 'and' should be 'an' - 'Create an agent and start it'
| // Create and agent and start it (temporary for now, will be merge later) | |
| // Create an agent and start it (temporary for now, will be merge later) |
src/core/pipeline_runner.rs
Outdated
| error: String, | ||
| } | ||
|
|
||
| /// Internal representation of a pipeline run, this should not be made public due to the fact that it contains |
There was a problem hiding this comment.
Missing comma after 'run' - should be 'Internal representation of a pipeline run, which should not be made public due to the fact that it contains'
| /// Internal representation of a pipeline run, this should not be made public due to the fact that it contains | |
| /// Internal representation of a pipeline run, which should not be made public due to the fact that it contains |
There was a problem hiding this comment.
I don't think it'd make sense for us to be sharing vscode settings in the repo? Let's be sure to remove this in a future PR.
Summary of changes from top to bottom in files changed:
Solve #103
Resolves PIPE-113
Resolves PIPE-116