Skip to content

Commit

Permalink
Fix wrong type_url in google-proto's Any type
Browse files Browse the repository at this point in the history
We were using the short url path, but the long url path is required.
  • Loading branch information
allada committed Apr 29, 2022
1 parent 717d87a commit 9cda96a
Show file tree
Hide file tree
Showing 3 changed files with 86 additions and 5 deletions.
13 changes: 13 additions & 0 deletions cas/scheduler/BUILD
Expand Up @@ -61,6 +61,19 @@ rust_library(
visibility = ["//cas:__pkg__", "//cas:__subpackages__"]
)

rust_test(
name = "action_messages_test",
srcs = ["tests/action_messages_test.rs"],
deps = [
"//proto",
"//third_party:pretty_assertions",
"//third_party:tokio",
"//util:common",
"//util:error",
":action_messages",
],
)

rust_test(
name = "scheduler_test",
srcs = ["tests/scheduler_test.rs"],
Expand Down
11 changes: 6 additions & 5 deletions cas/scheduler/action_messages.rs
Expand Up @@ -7,19 +7,20 @@ use std::hash::{Hash, Hasher};
use std::sync::Arc;
use std::time::{Duration, SystemTime};

use prost::Message;
use prost_types::Any;
use sha2::{Digest as _, Sha256};

use common::{DigestInfo, HashMapExt, VecExt};
use error::{make_input_err, Error, ResultExt};
use platform_property_manager::PlatformProperties;
use prost::Message;
use prost_types::Any;
use proto::build::bazel::remote::execution::v2::{
execution_stage, Action, ActionResult as ProtoActionResult, ExecuteOperationMetadata, ExecuteRequest,
ExecuteResponse, ExecutedActionMetadata, ExecutionPolicy, FileNode, LogFile, OutputDirectory, OutputFile,
OutputSymlink, Platform, SymlinkNode,
};
use proto::google::longrunning::{operation::Result as LongRunningResult, Operation};
use proto::google::rpc::Status;

/// This is a utility struct used to make it easier to match ActionInfos in a
/// HashMap without needing to construct an entire ActionInfo.
Expand Down Expand Up @@ -575,7 +576,7 @@ impl Into<ExecuteResponse> for ActionStage {
stderr_raw: Default::default(),
}),
cached_result: was_from_cache,
status: error.and_then(|v| Some(v.into())),
status: Some(error.map_or(Status::default(), |v| v.into())),
server_logs,
message: "TODO(blaise.bruer) We should put a reference something like bb_browser".to_string(),
}
Expand Down Expand Up @@ -660,12 +661,12 @@ impl Into<Operation> for ActionState {
Operation {
name: self.name,
metadata: Some(Any {
type_url: "build.bazel.remote.execution.v2.ExecuteOperationMetadata".to_string(),
type_url: "type.googleapis.com/build.bazel.remote.execution.v2.ExecuteOperationMetadata".to_string(),
value: metadata.encode_to_vec(),
}),
done: has_action_result,
result: Some(LongRunningResult::Response(Any {
type_url: "build.bazel.remote.execution.v2.ExecuteResponse".to_string(),
type_url: "type.googleapis.com/build.bazel.remote.execution.v2.ExecuteResponse".to_string(),
value: serialized_response,
})),
}
Expand Down
67 changes: 67 additions & 0 deletions cas/scheduler/tests/action_messages_test.rs
@@ -0,0 +1,67 @@
// Copyright 2022 Nathan (Blaise) Bruer. All rights reserved.

use std::time::SystemTime;

use action_messages::{ActionResult, ActionStage, ActionState, ExecutionMetadata};
use common::DigestInfo;
use error::Error;
use proto::build::bazel::remote::execution::v2::ExecuteResponse;
use proto::google::longrunning::{operation, Operation};
use proto::google::rpc::Status;

#[cfg(test)]
mod action_messages_tests {
use super::*;
use pretty_assertions::assert_eq; // Must be declared in every module.

#[tokio::test]
async fn action_state_any_url_test() -> Result<(), Error> {
let operation: Operation = ActionState {
name: "test".to_string(),
action_digest: DigestInfo::new([1u8; 32], 5),
stage: ActionStage::Unknown,
}
.into();

match operation.result {
Some(operation::Result::Response(any)) => assert_eq!(
any.type_url,
"type.googleapis.com/build.bazel.remote.execution.v2.ExecuteResponse"
),
other => assert!(false, "Expected Some(Result(Any)), got: {:?}", other),
}

Ok(())
}

#[tokio::test]
async fn execute_response_status_message_is_some_on_success_test() -> Result<(), Error> {
let execute_response: ExecuteResponse = ActionStage::Completed(ActionResult {
output_files: vec![],
output_folders: vec![],
output_symlinks: vec![],
exit_code: 0,
stdout_digest: DigestInfo::new([2u8; 32], 5),
stderr_digest: DigestInfo::new([3u8; 32], 5),
execution_metadata: ExecutionMetadata {
worker: "foo_worker_id".to_string(),
queued_timestamp: SystemTime::UNIX_EPOCH,
worker_start_timestamp: SystemTime::UNIX_EPOCH,
worker_completed_timestamp: SystemTime::UNIX_EPOCH,
input_fetch_start_timestamp: SystemTime::UNIX_EPOCH,
input_fetch_completed_timestamp: SystemTime::UNIX_EPOCH,
execution_start_timestamp: SystemTime::UNIX_EPOCH,
execution_completed_timestamp: SystemTime::UNIX_EPOCH,
output_upload_start_timestamp: SystemTime::UNIX_EPOCH,
output_upload_completed_timestamp: SystemTime::UNIX_EPOCH,
},
server_logs: Default::default(),
})
.into();

// This was once discovered to be None, which is why this test exists.
assert_eq!(execute_response.status, Some(Status::default()));

Ok(())
}
}

0 comments on commit 9cda96a

Please sign in to comment.