fastly / Viceroy

Viceroy provides local testing for developers working with Compute.
https://fastly.dev/learning/compute/testing/#running-a-local-testing-server
Apache License 2.0
140 stars 35 forks source link

Remove an unnecessary Arc from the Session #371

Closed elliottt closed 1 month ago

elliottt commented 1 month ago

ObjectStores internally is an Arc to an RwLock, so the outer Arc present in Session is not necessary. This PR removes that unnecessary indirection, as ObjectStore already derives Clone.

elliottt commented 1 month ago

LGTM. Rust idiomatic q: I notice that SecretStores has HashMap<String, SecretStore>, is one placement of the Arc preferable to the other?

I think it depends on what the desired external use of the value is: putting the Arc inside means that the value is clonable, while leaving it out means that the user of the type has more control over data indirection.

acw commented 1 month ago

LGTM. Rust idiomatic q: I notice that SecretStores has HashMap<String, SecretStore>, is one placement of the Arc preferable to the other?

I don't know if I've detected a general law, and I don't think clippy has never directed me in one direction or another. I can say that my general rule is:

athomason commented 1 month ago

Thanks!