8000 feat(dal): Respect subscriptions when calculating action dependencies by jkeiser · Pull Request #6102 · systeminit/si · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

feat(dal): Respect subscriptions when calculating action dependencies #6102

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.

Sign up for GitHub

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 8000 in to your account

Jump to bottom
Merged
merged 1 commit into from
May 13, 2025
Merged

feat(dal): Respect subscriptions when calculating action dependencies #6102

merged 1 commit into from
May 13, 2025

Conversation

jkeiser
Copy link
Contributor
@jkeiser jkeiser commented May 13, 2025 •
edited
Loading

This causes actions to run in the proper order when components depend on each other via subscription.

Fixes ENG-3019.

Tests

  • New integration tests
  • Manual test: create VPC and subnet with socket connections and make sure VPC goes first
  • Manual test: create VPC and subnet with subscriptions and make sure VPC goes first

@jkeiser jkeiser requested review from jhelwig and zacharyhamm May 13, 2025 00:36
@github-actions GitHub Actions
Copy link
github-actions bot commented May 13, 2025 •
edited
Loading

Dependency Review

✅ No vulnerabilities or OpenSSF Scorecard issues found.

Scanned Files

None

zacharyhamm
zacharyhamm reviewed May 13, 2025
View reviewed changes
Copy link
Contributor
@zacharyhamm zacharyhamm left a comment

Choose a reason for hiding this comment

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

Everything looks right to me. One request: a doc comment. Also, have you tested this with respect to the ordering of delete actions? I think those have been troublesome in the past.

lib/dal/src/component.rs
@@ -1321,6 +1322,14 @@ impl Component {
Ok(input_socket_ids)
}

pub async fn subscribers(
Copy link
Contributor

Choose a reason for hiding this comment

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

A doc comment here would be great, explaining what a Subscriber is

8000
@jkeiser
Copy link
Contributor Author
jkeiser commented May 13, 2025

Thanks!

I didn't directly test delete actions because I was reusing the dependency graph's reasoning for it; I should have. I've got some tests written for it, and will put them up shortly once I've checked that they, like, pass :)

@jkeiser jkeiser force-pushed the jkeiser/action-dependencies branch from 63c6af7 to 62f552b Compare May 13, 2025 15:35
@jkeiser jkeiser force-pushed the jkeiser/action-dependencies branch from 62f552b to bde0f5e Compare May 13, 2025 15:35
@jkeiser jkeiser added this pull request to the merge queue May 13, 2025
Merged via the queue into main with commit 5b42115 May 13, 2025
10 checks passed
@jkeiser jkeiser deleted the jkeiser/action-dependencies branch May 13, 2025 20:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jhelwig jhelwig jhelwig approved these changes

@zacharyhamm zacharyhamm zacharyhamm approved these changes

Assignees
No one assigned
Labels
A-dal A-dal-test
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jkeiser @jhelwig @zacharyhamm
0