Skip to content

execution: cache only successful action results #58

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
fionera wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

execution: cache only successful action results #58

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 30 additions & 5 deletions src/execution_service.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,8 @@ pub fn execute(
};
if (cached_entry) |*entry| {
defer entry.deinit(allocator);
return try completedOperation(allocator, action_digest, entry.result, true);
if (entry.result.exit_code == 0)
return try completedOperation(allocator, action_digest, entry.result, true);
}
}
}
Expand Down Expand Up @@ -90,7 +91,7 @@ pub fn execute(

var result = try action_executor.actionResultFromOutcomeOwned(allocator, outcome);
defer result.deinit(allocator);
return try publishSuccessfulOperation(
return try publishOperation(
io,
allocator,
result_store,
Expand All @@ -101,7 +102,7 @@ pub fn execute(
);
}

fn publishSuccessfulOperation(
fn publishOperation(
io: std.Io,
allocator: std.mem.Allocator,
result_store: ?action_cache.Store,
Expand All @@ -110,7 +111,7 @@ fn publishSuccessfulOperation(
do_not_cache: bool,
options: action_executor.ExecuteOptions,
) !CompletedOperation {
if (!do_not_cache) {
if (!do_not_cache and result.exit_code == 0) {
if (result_store) |store| {
if (executionCancelled(options)) {
return try completedFailedOperation(
Expand Down Expand Up @@ -325,7 +326,7 @@ test "cancelled action results are not written to the ActionCache" {
const store = action_cache.Store.init(tmp.dir);
const digest = cas.Digest.fromBytes("cancelled action result");
var cancellation = std.atomic.Value(bool).init(true);
var operation = try publishSuccessfulOperation(
var operation = try publishOperation(
std.testing.io,
std.testing.allocator,
store,
Expand All @@ -346,3 +347,27 @@ test "cancelled action results are not written to the ActionCache" {
store.get(std.testing.io, std.testing.allocator, digest),
);
}

test "failed action results are returned without entering the ActionCache" {
var tmp = std.testing.tmpDir(.{});
defer tmp.cleanup();
const store = action_cache.Store.init(tmp.dir);
for ([_]i32{ 1, 127, 137 }) |exit_code| {
const digest = cas.Digest.fromBytes("failed action result");
var operation = try publishOperation(
std.testing.io,
std.testing.allocator,
store,
digest,
.{ .exit_code = exit_code },
false,
.{},
);
defer operation.deinit(std.testing.allocator);
var reader = protobuf.Reader.init(operation.operation.response.?.value);
const response = try reapi.ExecuteResponse.decode(&reader);
try std.testing.expectEqual(exit_code, response.result.?.exit_code);
try std.testing.expect(!response.cached_result);
try std.testing.expectError(error.FileNotFound, store.get(std.testing.io, std.testing.allocator, digest));
}
}
Loading