Skip to content

Conversation

@mamcx
Copy link
Contributor

@mamcx mamcx commented Dec 15, 2025

Description of Changes

Closes #3659

API and ABI breaking changes

Remove route and alter the semantics of the call route on both server and cli

Expected complexity level and risk

1

Testing

  • Publish module with procedures and observe calling the cli the result is print.

@mamcx mamcx self-assigned this Dec 15, 2025
@mamcx mamcx added the api-break A PR that makes an API breaking change label Dec 15, 2025
Copy link
Contributor

@gefjon gefjon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks good. I tested locally and was able to call procedures. I'd like to see a smoketest added, please - something trivial that just calls a procedure and inspects its return value.

@bfops
Copy link
Collaborator

bfops commented Dec 18, 2025

It sounds like this is an API break - does that mean that older versions of the CLI will not be able to spacetime call to versions of the server that have this PR included?

.after_help("Run `spacetime help call` for more detailed information.\n")
}

enum CallResult<'a> {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it looks like this is just a description of what the call result will be, not the result itself, is that right?

fn add_reducer_ctx_to_err(error: &mut String, module_def: &ModuleDef, reducer_name: &str) {
let mut reducers = module_def
/// decorate `error` with more helpful info about reducers and procedures.
fn add_reducer_procedure_ctx_to_err(error: &mut String, module_def: &ModuleDef, reducer_name: &str) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

did you test some of these error cases as well? either manually or with a smoketest?

@gefjon
Copy link
Contributor

gefjon commented Dec 18, 2025

It sounds like this is an API break - does that mean that older versions of the CLI will not be able to spacetime call to versions of the server that have this PR included?

I don't think that's true. When calling reducers, the return format of the route is unchanged, to the best of my ability to determine. It's only when calling a procedure that the route returns something different, and the old CLI won't do that because it checks the moduledef first.

@bfops
Copy link
Collaborator

bfops commented Dec 19, 2025

It sounds like this is an API break - does that mean that older versions of the CLI will not be able to spacetime call to versions of the server that have this PR included?

I don't think that's true. When calling reducers, the return format of the route is unchanged, to the best of my ability to determine. It's only when calling a procedure that the route returns something different, and the old CLI won't do that because it checks the moduledef first.

Okay, great. I was just going off of this description:

Remove route and alter the semantics of the call route on both server and cli
And I assume the old route was unstable so it's fine to remove.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api-break A PR that makes an API breaking change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Procedures: Make /v1/database/:name/call/:func call procedures too, remove procedure route

4 participants