Skip to content

feat(core): dynamic runtime capability#9036

Merged
lucasfernog merged 4 commits intodevfrom
feat/dynamic-capability
Mar 1, 2024
Merged

feat(core): dynamic runtime capability#9036
lucasfernog merged 4 commits intodevfrom
feat/dynamic-capability

Conversation

@lucasfernog
Copy link
Member

Some capabilities must be dynamic (e.g. allow a dynamically started localhost server, runtime-defined remote URLs, fine-grained permissions that depend on user configuration like access token scopes..). This can be done already by creating a string at runtime and leaking it so it becomes a &'static str, but it's not ideal so we now support both runtime strings and added a new capability builder aswell so you can either include_str!() a static capability file or create/modify one at runtime.

@lucasfernog lucasfernog requested a review from a team as a code owner February 29, 2024 12:36
let identifier = permission
.clone()
.try_into()
.unwrap_or_else(|_| panic!("invalid permission identifier '{permission}'"));
Copy link
Member

Choose a reason for hiding this comment

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

I would try to avoid panicking and return a result instead

.into_iter()
.map(|a| {
serde_json::to_value(a)
.expect("failed to serialize scope")
Copy link
Member

Choose a reason for hiding this comment

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

ditto

.into_iter()
.map(|a| {
serde_json::to_value(a)
.expect("failed to serialize scope")
Copy link
Member

Choose a reason for hiding this comment

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

ditto

@lucasfernog
Copy link
Member Author

@amrbashir it's quite rare for those functions to panic (most identifiers are valid, and it's also almost impossible for serde_json::to_value to fail) and I don't really like the pattern of returning Result in a builder
plus we're also panicking on add_capability itself which I think is ok since if you mess up in the capability tou shouldn't ship your app at all

@amrbashir
Copy link
Member

That's fair I guess

@lucasfernog lucasfernog merged commit 03098b5 into dev Mar 1, 2024
@lucasfernog lucasfernog deleted the feat/dynamic-capability branch March 1, 2024 11:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants