From d38e6634d474c8c61564770729c10058d711fc2f Mon Sep 17 00:00:00 2001 From: Preslav Le Date: Thu, 9 May 2024 17:40:32 -0700 Subject: [PATCH] Remove block_logging from ActionCallbacks trait. (#25534) block_logging is very confusing. It is part of ActionCallbacks trait, which means it is serialized and deserialized from protos. However, all that obfuscates the fact that block_logging is always false for action callbacks. Thus the whole thing is redundant. To help clean this up, remove block_logging from the trait and set it to false. We could pass log_visibility into ApplicationFunctionRunner, so we can check if we should block visibility, but this will always return false since actions have AllowedVisibility::All anyway, so passing this and creating transactions in order to do the check seem like an overkill. GitOrigin-RevId: 3be542a7f2860e31b36a7506be1fa2054bb1b28a --- .../application/src/application_function_runner/mod.rs | 9 ++++++--- crates/isolate/src/client.rs | 3 --- crates/isolate/src/environment/action/async_syscall.rs | 3 --- crates/isolate/src/test_helpers.rs | 3 --- crates/pb/protos/backend.proto | 3 +++ 5 files changed, 9 insertions(+), 12 deletions(-) diff --git a/crates/application/src/application_function_runner/mod.rs b/crates/application/src/application_function_runner/mod.rs index ea932842..ec52e474 100644 --- a/crates/application/src/application_function_runner/mod.rs +++ b/crates/application/src/application_function_runner/mod.rs @@ -1954,9 +1954,10 @@ impl ActionCallbacks for ApplicationFunctionRunner { identity: Identity, name: UdfPath, args: Vec, - block_logging: bool, context: ExecutionContext, ) -> anyhow::Result { + // We never block logging for functions called by actions. + let block_logging = false; let ts = self.database.now_ts_for_reads(); let result = self .run_query_at_ts( @@ -1982,9 +1983,10 @@ impl ActionCallbacks for ApplicationFunctionRunner { identity: Identity, name: UdfPath, args: Vec, - block_logging: bool, context: ExecutionContext, ) -> anyhow::Result { + // We never block logging for functions called by actions. + let block_logging = false; let result = self .retry_mutation( context.request_id, @@ -2012,9 +2014,10 @@ impl ActionCallbacks for ApplicationFunctionRunner { identity: Identity, name: UdfPath, args: Vec, - block_logging: bool, context: ExecutionContext, ) -> anyhow::Result { + // We never block logging for functions called by actions. + let block_logging = false; let _tx = self.database.begin(identity.clone()).await?; let result = self .run_action( diff --git a/crates/isolate/src/client.rs b/crates/isolate/src/client.rs index bd04331e..81b5757e 100644 --- a/crates/isolate/src/client.rs +++ b/crates/isolate/src/client.rs @@ -275,7 +275,6 @@ pub trait ActionCallbacks: Send + Sync { identity: Identity, name: UdfPath, args: Vec, - block_logging: bool, context: ExecutionContext, ) -> anyhow::Result; @@ -284,7 +283,6 @@ pub trait ActionCallbacks: Send + Sync { identity: Identity, name: UdfPath, args: Vec, - block_logging: bool, context: ExecutionContext, ) -> anyhow::Result; @@ -293,7 +291,6 @@ pub trait ActionCallbacks: Send + Sync { identity: Identity, name: UdfPath, args: Vec, - block_logging: bool, context: ExecutionContext, ) -> anyhow::Result; diff --git a/crates/isolate/src/environment/action/async_syscall.rs b/crates/isolate/src/environment/action/async_syscall.rs index 604728e1..98ac6ec9 100644 --- a/crates/isolate/src/environment/action/async_syscall.rs +++ b/crates/isolate/src/environment/action/async_syscall.rs @@ -93,7 +93,6 @@ impl TaskExecutor { self.identity.clone(), udf_path, args.into_arg_vec(), - false, self.context.clone(), ) .await? @@ -124,7 +123,6 @@ impl TaskExecutor { self.identity.clone(), udf_path, args.into_arg_vec(), - false, self.context.clone(), ) .await? @@ -152,7 +150,6 @@ impl TaskExecutor { self.identity.clone(), udf_path, args.into_arg_vec(), - false, self.context.clone(), ) .await? diff --git a/crates/isolate/src/test_helpers.rs b/crates/isolate/src/test_helpers.rs index f81cb44a..44e30a3e 100644 --- a/crates/isolate/src/test_helpers.rs +++ b/crates/isolate/src/test_helpers.rs @@ -1146,7 +1146,6 @@ impl ActionCallbacks for UdfTest { identity: Identity, name: UdfPath, args: Vec, - _block_logging: bool, _context: ExecutionContext, ) -> anyhow::Result { let arguments = parse_udf_args(&name, args)?; @@ -1167,7 +1166,6 @@ impl ActionCallbacks for UdfTest { identity: Identity, name: UdfPath, args: Vec, - _block_logging: bool, _context: ExecutionContext, ) -> anyhow::Result { let arguments = parse_udf_args(&name, args)?; @@ -1188,7 +1186,6 @@ impl ActionCallbacks for UdfTest { identity: Identity, name: UdfPath, args: Vec, - _block_logging: bool, _context: ExecutionContext, ) -> anyhow::Result { let arguments = parse_udf_args(&name, args)?; diff --git a/crates/pb/protos/backend.proto b/crates/pb/protos/backend.proto index 9e8dd8a9..ff0071e8 100644 --- a/crates/pb/protos/backend.proto +++ b/crates/pb/protos/backend.proto @@ -71,6 +71,7 @@ message ExecuteQueryRequest { convex_identity.UncheckedIdentity identity = 2; common.PathAndArgs path_and_args = 3; + // TODO(presley): Delete optional bool block_logging = 5; optional common.ExecutionContext execution_context = 6; @@ -89,6 +90,7 @@ message ExecuteMutationRequest { convex_identity.UncheckedIdentity identity = 2; common.PathAndArgs path_and_args = 3; + // TODO(presley): Delete optional bool block_logging = 5; optional common.ExecutionContext execution_context = 6; @@ -107,6 +109,7 @@ message ExecuteActionRequest { convex_identity.UncheckedIdentity identity = 2; common.PathAndArgs path_and_args = 3; + // TODO(presley): Delete optional bool block_logging = 5; optional common.ExecutionContext execution_context = 6;