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.

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

Jump to bottom

Initial refactor for 'no fail' actor start #4

Merged
merged 4 commits into from
Nov 13, 2020
Merged

Initial refactor for 'no fail' actor start #4

merged 4 commits into from
Nov 13, 2020

Conversation

autodidaddict
Copy link
Member

After talking to the people who actually know what they're doing in the Actix gitter/discord, I found that I was using a bit of an anti-pattern. It should be impossible for an actor to fail to start, and there were a couple of places, especially in the capability provider host actor, where I was pulling shenanigans like try_new(), which I then hid behind a builder (that also returned a result).

Instead, the recommended practice is that all actors should be 100% guaranteed to start, and if you need to set an initial state that could be rejected or invalid, then you do so through sending that message to the actor, and the actor stops itself if there's an error.

So far I've only refactored the host controller's interaction with the native capability provider host. After we agree on the cleanest way to implement this, I can poke around the rest of all the actors and make them conform to this new pattern.

@autodidaddict
Removing potentially lethal unwraps from actor start sequence - refac…
76e5135 
…tored to send new actors an initialize message
@autodidaddict
Copy link
Member Author

@brooksmtownsend just pushed a new batch that removes potentially panic-inducing unwraps from the actor startup, adopting the new "optional state with initializer" pattern.

brooksmtownsend
brooksmtownsend requested changes Nov 12, 2020
View reviewed changes
Copy link
Member

@brooksmtownsend brooksmtownsend left a comment

Choose a reason for hiding this comment

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

I might be taking the "Actor should always be able to start" too literally, I don't want to encourage creating weird program state to satisfy that. I am interested especially in the actor host Initialize handler about what this should look like.

crates/wascc-host/src/actors/actor_host.rs Show resolved Hide resolved
crates/wascc-host/src/actors/actor_host.rs Show resolved Hide resolved
crates/wascc-host/src/actors/actor_host.rs Outdated Show resolved Hide resolved
brooksmtownsend
brooksmtownsend approved these changes Nov 12, 2020
View reviewed changes
Copy link
Member

@brooksmtownsend brooksmtownsend left a comment

Choose a reason for hiding this comment

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

Those code pieces look a lot cleaner without the header referenced constantly.

@autodidaddict autodidaddict merged commit 2ed73d6 into main Nov 13, 2020
@autodidaddict autodidaddict deleted the nofailactor branch November 13, 2020 15:03
brooksmtownsend added a commit that referenced this pull request May 4, 2021
@brooksmtownsend
Archive providers using Tar rather than Zip (#4)
a24060f 
* changed zip compression method to stored to prevent lossy compression

* remove debug derive, remove extra clone

* revoked public struct fields

* Replaced `zip` library with `tar` for consistent archival of par files

* Increased version number

* Changed try_load to accept byte array, renamed test archives
brooksmtownsend added a commit that referenced this pull request May 4, 2021
@brooksmtownsend @autodidaddict @ChrisRx
Add provider-archive to the crates/ directory (#191)
5cc74ce 
* Initial commit

* Update README.md (#1)

Furthering the cause of DDD - documentation-driven-development.

* Initial implementation (#2)

* Initial implementation

* Adding function to retrieve target bytes

* Adding cargo.lock

* Archive providers using Tar rather than Zip (#4)

* changed zip compression method to stored to prevent lossy compression

* remove debug derive, remove extra clone

* revoked public struct fields

* Replaced `zip` library with `tar` for consistent archival of par files

* Increased version number

* Changed try_load to accept byte array, renamed test archives

* Making the result type error Sync and Send

* Add rust build action

* Created release action for crates.io

* Support par compression (#5)

* changed zip compression method to stored to prevent lossy compression

* remove debug derive, remove extra clone

* revoked public struct fields

* Replaced `zip` library with `tar` for consistent archival of par files

* Increased version number

* Changed try_load to accept byte array, renamed test archives

* added ability to compress and decompress par archives

* use result

* changed compression algorithm to gz, included compression tests, bumped to 0.2.0

* refined tests to compare raw data rather than length of bytes

* refined another test

* removed trailing parenthesis

* Detect gzip file via magic header

Co-authored-by: chris <[email protected]>

* Updated zip references to tar in README

* Update README.md

Adding build badge

* Update README.md

Adding other badges

* Update README.md

* updated release action

* changed write to either output compressed or uncompressed par, not both (#6)

* Fixes #7, write par file only once

This fixes issue #7 with the gz suffix being added to the archive
even if it already exists. It also changes how the par file is written
in cases where gzip compression is used. Previously, it would write the
file then read that in and write the compressed file, where as now it
just writes the par file once, regardless of compression.

Cleanup try_load compression usage, move compress/decompress to tests

Bump minor version

* added rev and ver to claims preserved on load (#11)

* Wascap bump (#12)

* bumped wascap version

* bumped wascap to 0.5.2

* bump dependencies for wascap update (#13)

* temporarily remove conflicts

* complete merge

* added provider-archive to workspace

Co-authored-by: Kevin Hoffman <[email protected]>
Co-authored-by: Kevin Hoffman <[email protected]>
Co-authored-by: chris <[email protected]>
rvolosatovs referenced this pull request in rvolosatovs/wasmCloud Mar 21, 2023
@based-template @dyn-trait
updated dependencies (#4)
4a0b018 
Co-authored-by: Alexander Schnackenberg <[email protected]>
rvolosatovs referenced this pull request in rvolosatovs/wasmCloud Mar 21, 2023
@autodidaddict
Closes #2 - Sanitizes container and blob IDs so that they can't be us…
754413d 
…ed to view content outside the FS provider's root path (#4)
rvolosatovs referenced this pull request in rvolosatovs/wasmCloud Mar 21, 2023
@protochron
Add initial provider implementation (#4)
748221d 
This implements the provider by leaning on the reqwests library for
handling all of the HTTP details and tokio for making it async.

Closes #1.
rvolosatovs referenced this pull request in rvolosatovs/wasmCloud Mar 21, 2023
@protochron
Change package name and update header (#7)
c8b2bc5 
Small fixes from #4.
rvolosatovs referenced this pull request in rvolosatovs/wasmCloud Mar 21, 2023
@Atul9
Add cargo fmt to github actions (#4)
1e66548
rvolosatovs referenced this pull request in rvolosatovs/wasmCloud Mar 21, 2023
@based-template
Updated to codec version 0.8 (#4)
921a157
rvolosatovs referenced this pull request in rvolosatovs/wasmCloud Mar 21, 2023
@brooksmtownsend
Merge pull request #4 from brooksmtownsend/main
413a558 
corrected cargo clippy working dir, not using action
connorsmith256 pushed a commit to connorsmith256/wasmCloud that referenced this pull request Oct 17, 2023
@brooksmtownsend
wash par implementation (wasmCloud#4)
c215ff2 
* Migrated functionality for claims from wascap crate

* Migrated functionality for keys from nkeys crate

* Migrated functionality for lattice from latticeclient crate

* Updated CLI to support keys, claims, and lattice commands

* Added par subcommand, created ability for auto-generating keys during par signing

* pinned dependencies, beautified wash par inspect output, updated par README

* Bumped version

* created result error type for par module

* changed generated message

Co-authored-by: Brooks Townsend <[email protected]>
connorsmith256 pushed a commit to connorsmith256/wasmCloud that referenced this pull request Oct 17, 2023
@brooksmtownsend
wash par implementation (wasmCloud#4)
c8cb66f 
* Migrated functionality for claims from wascap crate

* Migrated functionality for keys from nkeys crate

* Migrated functionality for lattice from latticeclient crate

* Updated CLI to support keys, claims, and lattice commands

* Added par subcommand, created ability for auto-generating keys during par signing

* pinned dependencies, beautified wash par inspect output, updated par README

* Bumped version

* created result error type for par module

* changed generated message

Co-authored-by: Brooks Townsend <[email protected]>
connorsmith256 pushed a commit that referenced this pull request Oct 19, 2023
@autodidaddict
Merge pull request #4 from bketelsen/add-rev-version
d1be595 
Add Revision and Version support
This was referenced Oct 30, 2023
Merged
Closed
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@brooksmtownsend brooksmtownsend brooksmtownsend approved these changes

@billyoung billyoung Awaiting requested review from billyoung

@ChrisRx ChrisRx Awaiting requested review from ChrisRx

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@autodidaddict @brooksmtownsend