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?

@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.

@mamcx mamcx force-pushed the mamcx/call-func-procedures branch from 87ca20f to c5dfb0c Compare December 25, 2025 14:49
Copy link
Collaborator

@jdetter jdetter left a comment

Choose a reason for hiding this comment

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

Optional nits, seems fine to me. I have not manually tested this but it looks like we've added some smoketests so I'm happy with that 👍

@gefjon gefjon enabled auto-merge December 31, 2025 22:17
@gefjon gefjon added this pull request to the merge queue Dec 31, 2025
Merged via the queue into master with commit 0386222 Jan 1, 2026
40 of 42 checks passed
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

5 participants