Logging features for during and post run

[Synicix]

Added:

• get_logs() and get_logs_blocking() that query the logs from the docker API
• Add status Undefined for status to deal with docker api possible return status of restarting, removing, paused, and dead
• Change basic_test to be only asserts, and added excuter_wrapper to handle fixture scaffolding
• Fix basic test assert to list by pod_run as discussed on Friday to avoid collisions
• Add a basic test for log functionality.
• Log are save as a log.txt when saved to disk, and is read back into memory upon loading
• Added hash updating to the logging test.

[codecov]

Codecov Report

❌ Patch coverage is 80.76923% with 15 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
src/uniffi/orchestrator/docker.rs	79.03%	13 Missing ⚠️
src/core/error.rs	0.00%	1 Missing ⚠️
src/core/orchestrator/docker.rs	0.00%	1 Missing ⚠️

📢 Thoughts on this report? Let us know!

[guzman-raphael]

@Synicix

• Seems like this is an independent feature. Could you remove the dependency on #72?
• It is missing saving the logs to a separate, single file when storing. We discussed that during YAML serialization, the logs fields should specify the hard-coded, relative path of the log file (e.g. output.log) and it should be skipped when hashing. If you depend on #90, then you can skip hashing in to_yaml(..).

[Synicix]

@guzman-raphael
Ah forgot about that feature we discussed. I can quickly added it today.

As for the dependencies, are we able to leave it as that since I expect the 72 to be merge first?

[guzman-raphael]

@Synicix Sounds good.

The reason I'm leaning towards separating them is because I am prioritizing the features included in our current milestone + anything that is fundamentally broken. Just trying to make sure we deliver on schedule.

My understanding was that #72 addresses some minor improvements around communicating better error messages for a better UX but doesn't address critical, show-stopper bugs (if I'm mistaken about that, please let me know).

[Synicix]

@​guzman-raphael

You're right—it doesn't address any major bugs, but it's been a pull request that's been sitting for a few months now, so I thought it made sense to go ahead and merge it.

Also, feel free to correct me if I'm mistaken, but I interpreted your response on pull request #72 as indicating that you'd already reviewed it and were just suggesting that the logging changes be moved to a separate PR. It's possible I misunderstood what you meant.

[Synicix]

Added

• Pod result save logs to log.txt now, and loads from it when calling from store.

[Synicix]

For eaiser review in the mean time we decide on whether to remove #72 dependencies or not:
Synicix/orcapod@failure_modes...Synicix:orcapod:logging

[Synicix]

@guzman-raphael Removed dependencies on #72

Updated the initial commit with a summary of everything I changed.
Should be much cleaner to merge now.

[Copilot]

Pull Request Overview

This PR introduces logging functionality during and after container runs and refines test scaffolding for better clarity and collision handling. Key changes include the addition of get_logs() and get_logs_blocking() functions querying Docker logs, updates to the Status enum with new variants (Queued and Undefined), and modifications to test functions (renaming basic_test to execute_wrapper and adapting logging tests).

Reviewed Changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
tests/orchestrator.rs	Renamed test wrapper and updated assertions for improved testing
tests/fixture/mod.rs	Added example logs value for pod results
src/uniffi/store/filestore.rs	Added saving and loading logs in separate file
src/uniffi/orchestrator/mod.rs	Updated Status enum and added log retrieval functions
src/uniffi/orchestrator/docker.rs	Implemented get_logs with aggregation from stdout and stderr
src/uniffi/model.rs	Modified PodResult to include logs field and adjusted its construction
src/uniffi/error.rs	Added error variant for failed run info extraction
src/core/store/filestore.rs	Made save_file function pub(crate)
src/core/orchestrator/docker.rs	Changed the default mapping for container status to Undefined
src/core/model.rs	Updated to_yaml to skip the logs field
src/core/error.rs	Updated error formatting to include the new error variant

Comments suppressed due to low confidence (1)

src/uniffi/model.rs:221

• [nitpick] Consider removing or clarifying the '// Skip' comment to clearly explain the purpose of the assigned_name parameter and its usage within PodResult.

        assigned_name: String, // Skip

[Synicix]

• Added hash updating to the logging test.

[Synicix]

Close do to too many merge conflicts