Skip to content

Conversation

@apascal07
Copy link
Collaborator

@apascal07 apascal07 commented Aug 16, 2024

Fixes #391.

Instead of:

const myFlow = defineFlow(...);
const output = await runFlow(myFlow, ...);

const myStreamingFlow = defineFlow(...);
const { output, stream } = await streamFlow(myStreamingFlow, ...);

for await (const chunk of stream()) {
  console.log(chunk);
}

console.log(await output());

You will now do:

const myFlow = defineFlow(...);
const result = myFlow(...);

const myStreamingFlow = defineStreamingFlow(...);
const { stream, output } = await myStreamingFlow(...);

// `stream` and `output` are now values, not functions.

for await (const chunk of stream) {
  console.log(chunk);
}

console.log(await output);

Checklist (if applicable):

  • Tested (manually, unit tested, etc.)
  • Changelog updated
  • Docs updated
@apascal07 apascal07 linked an issue Aug 16, 2024 that may be closed by this pull request
@apascal07 apascal07 changed the title Split out defineFlow into itself + defineStreamingFlow with callable interfaces for both. Aug 16, 2024
@apascal07 apascal07 requested a review from mbleigh August 16, 2024 21:15
@apascal07 apascal07 marked this pull request as ready for review August 16, 2024 21:15
@apascal07 apascal07 requested a review from pavelgj August 16, 2024 21:15
@pavelgj
Copy link
Collaborator

pavelgj commented Aug 28, 2024

could you please summarize API changes in the PR description?

@apascal07
Copy link
Collaborator Author

could you please summarize API changes in the PR description?

Done.

@pavelgj
Copy link
Collaborator

pavelgj commented Sep 5, 2024

ugh, I forgot about this PR... and now I have a change that rips our durable flows, that massively conflicts with this...

@pavelgj
Copy link
Collaborator

pavelgj commented Sep 5, 2024

could you please summarize API changes in the PR description?

Done.

I don't remember, have we discussed this API at some point ([in]formal review with the team)?

@apascal07
Copy link
Collaborator Author

apascal07 commented Sep 5, 2024

could you please summarize API changes in the PR description?

Done.

I don't remember, have we discussed this API at some point ([in]formal review with the team)?

Chris filed this issue (#391) with the resolution based on a discussion where at least you, me, and Michael were involved.

@pavelgj
Copy link
Collaborator

pavelgj commented Sep 5, 2024

could you please summarize API changes in the PR description?

Done.

I don't remember, have we discussed this API at some point ([in]formal review with the team)?

Chris filed this issue (#391) with the resolution based on a discussion where at least you, me, and Michael were involved.

Ah, I found the discussion. I am a bit torn on splitting out defineFlow and defineStreamingFlow... but I think it might simplify a few thing... so, yeah, LGTM.

@apascal07 apascal07 merged commit 5f21198 into next Sep 5, 2024
@apascal07 apascal07 deleted the ap-js-callables branch September 5, 2024 15:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants