Skip to content

Commit

Permalink
Remove block_logging from ActionCallbacks trait. (#25534)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
preslavle authored and Convex, Inc. committed May 10, 2024
1 parent f84d827 commit d38e663
Show file tree
Hide file tree
Showing 5 changed files with 9 additions and 12 deletions.
9 changes: 6 additions & 3 deletions crates/application/src/application_function_runner/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1954,9 +1954,10 @@ impl<RT: Runtime> ActionCallbacks for ApplicationFunctionRunner<RT> {
identity: Identity,
name: UdfPath,
args: Vec<JsonValue>,
block_logging: bool,
context: ExecutionContext,
) -> anyhow::Result<FunctionResult> {
// 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(
Expand All @@ -1982,9 +1983,10 @@ impl<RT: Runtime> ActionCallbacks for ApplicationFunctionRunner<RT> {
identity: Identity,
name: UdfPath,
args: Vec<JsonValue>,
block_logging: bool,
context: ExecutionContext,
) -> anyhow::Result<FunctionResult> {
// We never block logging for functions called by actions.
let block_logging = false;
let result = self
.retry_mutation(
context.request_id,
Expand Down Expand Up @@ -2012,9 +2014,10 @@ impl<RT: Runtime> ActionCallbacks for ApplicationFunctionRunner<RT> {
identity: Identity,
name: UdfPath,
args: Vec<JsonValue>,
block_logging: bool,
context: ExecutionContext,
) -> anyhow::Result<FunctionResult> {
// 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(
Expand Down
3 changes: 0 additions & 3 deletions crates/isolate/src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,6 @@ pub trait ActionCallbacks: Send + Sync {
identity: Identity,
name: UdfPath,
args: Vec<JsonValue>,
block_logging: bool,
context: ExecutionContext,
) -> anyhow::Result<FunctionResult>;

Expand All @@ -284,7 +283,6 @@ pub trait ActionCallbacks: Send + Sync {
identity: Identity,
name: UdfPath,
args: Vec<JsonValue>,
block_logging: bool,
context: ExecutionContext,
) -> anyhow::Result<FunctionResult>;

Expand All @@ -293,7 +291,6 @@ pub trait ActionCallbacks: Send + Sync {
identity: Identity,
name: UdfPath,
args: Vec<JsonValue>,
block_logging: bool,
context: ExecutionContext,
) -> anyhow::Result<FunctionResult>;

Expand Down
3 changes: 0 additions & 3 deletions crates/isolate/src/environment/action/async_syscall.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,6 @@ impl<RT: Runtime> TaskExecutor<RT> {
self.identity.clone(),
udf_path,
args.into_arg_vec(),
false,
self.context.clone(),
)
.await?
Expand Down Expand Up @@ -124,7 +123,6 @@ impl<RT: Runtime> TaskExecutor<RT> {
self.identity.clone(),
udf_path,
args.into_arg_vec(),
false,
self.context.clone(),
)
.await?
Expand Down Expand Up @@ -152,7 +150,6 @@ impl<RT: Runtime> TaskExecutor<RT> {
self.identity.clone(),
udf_path,
args.into_arg_vec(),
false,
self.context.clone(),
)
.await?
Expand Down
3 changes: 0 additions & 3 deletions crates/isolate/src/test_helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1146,7 +1146,6 @@ impl<RT: Runtime, P: Persistence + Clone> ActionCallbacks for UdfTest<RT, P> {
identity: Identity,
name: UdfPath,
args: Vec<JsonValue>,
_block_logging: bool,
_context: ExecutionContext,
) -> anyhow::Result<FunctionResult> {
let arguments = parse_udf_args(&name, args)?;
Expand All @@ -1167,7 +1166,6 @@ impl<RT: Runtime, P: Persistence + Clone> ActionCallbacks for UdfTest<RT, P> {
identity: Identity,
name: UdfPath,
args: Vec<JsonValue>,
_block_logging: bool,
_context: ExecutionContext,
) -> anyhow::Result<FunctionResult> {
let arguments = parse_udf_args(&name, args)?;
Expand All @@ -1188,7 +1186,6 @@ impl<RT: Runtime, P: Persistence + Clone> ActionCallbacks for UdfTest<RT, P> {
identity: Identity,
name: UdfPath,
args: Vec<JsonValue>,
_block_logging: bool,
_context: ExecutionContext,
) -> anyhow::Result<FunctionResult> {
let arguments = parse_udf_args(&name, args)?;
Expand Down
3 changes: 3 additions & 0 deletions crates/pb/protos/backend.proto
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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;

Expand All @@ -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;

Expand Down

0 comments on commit d38e663

Please sign in to comment.