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

#15: Publish multiple commands #16

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

apetkov-so
Copy link
Contributor

  • The VM now Yields to the runtime when it publishes a command. The runtime is responsible for sealing the command, updating the facts database, then resuming the VM so it can process the next command.
  • FFI calls now take an immutable self.

@apetkov-so
Copy link
Contributor Author

Add test that publishes multiple types of commands (per @gknopf-aranya)

map F[] as f {
  publish A {}
  publish B {}
}

@apetkov-so apetkov-so added the vm Aranya policy lang VM label Jan 7, 2025
@apetkov-so apetkov-so force-pushed the 15-publish-multiple-commands-from-action branch from 2dcc829 to 726b3b2 Compare January 8, 2025 19:51
Copy link
Contributor

@chip-so chip-so left a comment

Choose a reason for hiding this comment

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

Otherwise, this looks good.

crates/aranya-policy-vm/src/machine.rs Outdated Show resolved Hide resolved
crates/aranya-policy-vm/src/machine.rs Outdated Show resolved Hide resolved
- The VM now `Yield`s to the  runtime when it publishes a command. The runtime is responsible for sealing the command, updating the facts database, then resuming the VM so it can process the next command.
- FFI `call`s now take an immutable self.
…nState (to potentially publish more). But the context's head_id needed to be updated to the new command's ID - this is now being done. That was difficult with the context being a reference, because the new context doesn't live long enough. So the RunState now owns the context.
@apetkov-so apetkov-so force-pushed the 15-publish-multiple-commands-from-action branch from c971a30 to 74ded88 Compare January 15, 2025 23:51
@apetkov-so apetkov-so force-pushed the 15-publish-multiple-commands-from-action branch from 4c3afc1 to cd96acd Compare January 16, 2025 20:05
@apetkov-so apetkov-so force-pushed the 15-publish-multiple-commands-from-action branch from cd96acd to cc082d8 Compare January 16, 2025 20:26
Copy link

@ssweetney-spok ssweetney-spok left a comment

Choose a reason for hiding this comment

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

Looks good!

Copy link
Contributor

@chip-so chip-so left a comment

Choose a reason for hiding this comment

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

This looks good otherwise!

@@ -1414,6 +1414,8 @@ fn should_create_clients_with_args() {
assert_eq!(effects, expected);
}

/// Test for <https://git.spideroak-inc.com/spideroak-inc/flow3-rs/issues/917>.
Copy link
Contributor

Choose a reason for hiding this comment

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

Since we're working in the public repo, this should probably point to a public issue or just refer to the issue without a URL.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
vm Aranya policy lang VM
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Publish multiple commands from action
3 participants