Skip to content

Conversation

@guzman-raphael
Copy link
Contributor

@guzman-raphael guzman-raphael commented Jul 29, 2025

Feature

Housekeeping

  • Create an alias for Packet.
  • Rename Status -> PodStatus to give a clear distinction once we introduce PipelineStatus.
  • Rename RunInfo -> PodRunInfo to give a clear distinction once we introduce PipelineRunInfo.
  • Box external errors to reduce size of OrcaError.
  • Clean up EventMetadata and remove event_classifier from start_service(..).
  • Make agent regex more flexible.
  • Remove print to console from agent client log(..).
  • Simplify store regex.

Docs

  • Update some intra-doc links.

Tests

  • Use separate network namespace for some error tests.
  • Stream console output when running tests with VSCode inline test GUI.
  • Add note about reusing cache when running tests with coverage i.e. build faster.

@codecov
Copy link

codecov bot commented Jul 29, 2025

Codecov Report

❌ Patch coverage is 94.17476% with 6 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/uniffi/model/pod.rs 88.88% 4 Missing ⚠️
src/uniffi/orchestrator/agent.rs 66.66% 2 Missing ⚠️

📢 Thoughts on this report? Let us know!

…and add note about reusing cache when running tests with coverage i.e. build faster.
@guzman-raphael guzman-raphael marked this pull request as ready for review July 30, 2025 02:17
@guzman-raphael guzman-raphael requested a review from Synicix July 30, 2025 02:17
@Synicix
Copy link
Contributor

Synicix commented Jul 30, 2025

  • Box external errors to reduce size of OrcaError. -> I thought we decided to force everything to be below the box requirement limit of clippy?

  • Rename RunInfo -> PodRunInfo to give a clear distinction once we introduce PipelineRunInfo. -> We got rid of PipelineRunInfo entirely and merge it with PipelineRun. The user will get a hash to access to talk to the runner.

@guzman-raphael
Copy link
Contributor Author

Box external errors to reduce size of OrcaError. -> I thought we decided to force everything to be below the box requirement limit of clippy?

I spent some more time on this since it came up elsewhere. Turns out that the clippy suggestion is actually a good one and it is about understanding how memory is allocated to the stack for enums. Basically, by using box for external errors, we cap out the memory per variant to a pointer to the heap which is just 8 bytes which overall should reduce the memory of our OrcaError for most cases . Here's an example on rust playground that demonstrates what I mean.

Rename RunInfo -> PodRunInfo to give a clear distinction once we introduce PipelineRunInfo. -> We got rid of PipelineRunInfo entirely and merge it with PipelineRun. The user will get a hash to access to talk to the runner.

I was doing some more thinking on this and there is a distinction we should make that we can use PipelineRunInfo for. Internally, we can still keep all the pipeline run data together like we discussed but offer 2 modes for user to access it: static vs dynamic.

Static being the data that doesn't change for the life of the pipeline run (e.g. assigned_name, created time, etc.). These would be acceptable to return in a PipelineRun.

Dynamic being the data that changes while it is progressing (e.g. node states, terminated time, packets completed per node, etc.). This is where we can do something similar to the orchestrator (perhaps agent_client.get_pipeline_info(pipeline_run)) to grab the state at a specific point in time. Therefore, I see value now in using PipelineRunInfo for that.

In summary, PipelineRun and PipelineRunInfo are each subsets of a complete pipeline run in memory. Perhaps the internal type that stores it all can be called PipelineRunState or TrackedPipelineRun.

Copy link
Contributor

@Synicix Synicix left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good

@Synicix Synicix merged commit 80c09a6 into nauticalab:dev Aug 4, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add output_packet to Pod Result

2 participants