Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

calling flows from flows and other support for 9796 #10024

Merged
merged 10 commits into from
Sep 5, 2024
Merged

Conversation

turadg
Copy link
Member

@turadg turadg commented Sep 4, 2024

refs: #9796

Description

#10023 started with a series of refactors and cleanup. This separates those into another PR to be reviewed more rapidly.

It also includes a new feature to orchestrateAll to ease calling flows from flows. Each flow is available on the context object under flows.

Security Considerations

Every flow can call another flow. In the event a developer wants isolate between flows, they won't put them in the same orchestrateAll.

Scaling Considerations

n/a

Documentation Considerations

none

Testing Considerations

new tests

Upgrade Considerations

not yet deployed

Copy link

cloudflare-workers-and-pages bot commented Sep 4, 2024

Deploying agoric-sdk with  Cloudflare Pages  Cloudflare Pages

Latest commit: 84d3ea6
Status: ✅  Deploy successful!
Preview URL: https://c2f4b9b2.agoric-sdk.pages.dev
Branch Preview URL: https://9796-support.agoric-sdk.pages.dev

View logs

Copy link
Member

@0xpatrickdev 0xpatrickdev left a comment

Choose a reason for hiding this comment

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

LGTM - please consider the Proxying -> Proxy name change

packages/async-flow/src/endowments.js Show resolved Hide resolved
Comment on lines +27 to +28
// Osmosis is one of the few chains with icqEnabled
const osmosis = await orch.getChain('osmosis');
Copy link
Member

Choose a reason for hiding this comment

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

omniflix appears to have icq enabled, but that's not to suggest we should revert this change. osmosis is preferable from my POV since it's integrated in multichain-testing - starship doesn't yet support omniflix.

@@ -43,7 +43,7 @@ const preparePortfolioHolderKit = (zone, { asVow, when }) => {
'PortfolioHolderKit',
{
invitationMakers: M.interface('InvitationMakers', {
MakeInvitation: M.call(
Proxying: M.call(
Copy link
Member

Choose a reason for hiding this comment

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

Wb Proxy, verb, instead of a present participle?

Copy link
Member Author

Choose a reason for hiding this comment

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

That was my first thought too but since Proxy is a global object I thought it would be confusing.

mhofman
mhofman previously requested changes Sep 4, 2024
packages/orchestration/src/facade.js Outdated Show resolved Hide resolved
@turadg turadg requested a review from mhofman September 4, 2024 19:13
packages/internal/src/utils.js Show resolved Hide resolved
packages/internal/test/utils.test.js Outdated Show resolved Hide resolved
packages/orchestration/src/facade.js Outdated Show resolved Hide resolved
@turadg turadg added the automerge:rebase Automatically rebase updates, then merge label Sep 5, 2024
Copy link
Member

@mhofman mhofman left a comment

Choose a reason for hiding this comment

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

I added some commits to enable mapping guest flows anywhere in the context object and updated a test accordingly.

@mergify mergify bot merged commit bd19a6f into master Sep 5, 2024
80 checks passed
@mergify mergify bot deleted the 9796-support branch September 5, 2024 19:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge:rebase Automatically rebase updates, then merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants