Rename config to variables #1918

fibonacci1729 · 2023-10-19T17:29:11Z

Renames environment variables SPIN_CONFIG_* to SPIN_VARIABLE_* for the EnvVariablesProvider.
Renames runtime config manifest key from config_provider to variables_provider.
Moves crates/config to crates/variables.
All tests/examples updated.

fibonacci1729 · 2023-10-19T17:31:57Z

An annoying detail: the environment runtime variable provider is now confusingly named EnvVariableProvider. Open to naming suggestions if this irks others as well.

crates/trigger/src/runtime_config/variable_provider.rs

Cargo.toml

crates/trigger/src/runtime_config.rs

crates/variables/src/lib.rs

crates/trigger/src/runtime_config/variable_provider.rs

crates/trigger/src/runtime_config.rs

rylev

Looking good!! I have some small things I think we should address before we merge this though.

crates/variables/src/host_component.rs

examples/config-rust/.env

examples/spin-timer/deps/spin/variables.wit

rylev · 2023-10-20T08:22:49Z

examples/spin-timer/deps/spin/variables.wit

+        invalid-name(string),
+        provider(string),
+        other(string),
+    }


Do we want to have a not-found error for when the requested variable is not found? Right now this gets encoded as a provider error which doesn't seem right.

I left it out because I didn't want to imply that not-found was a "normal" condition (like the v1 key-value no-such-key); not being able to resolve a variable is a programming or operational mistake.

It's strange that this error is labeled as a provider error when it's fairly likely not a provider error. I think the most likely cause will be users thinking there is a variable name foo and there not actually being one.

It's not a huge deal that this gets rendered as provider("no variable named 'foo'") - the error is clear enough - it's just a bit strange is all.

I personally don't really see potential confusion from having another error case, but I guess I'm fine letting this go if everyone disagrees with me.

I added a variant undefined to specifically refer to this case @rylev. Could you take another look at the code and let me know what you think?

sdk/rust/src/lib.rs

examples/spin-timer/deps/spin/variables.wit

crates/variables/src/provider/env.rs

rylev · 2023-10-23T09:23:00Z

examples/spin-timer/deps/spin/variables.wit

+        invalid-name(string),
+        provider(string),
+        other(string),
+    }


It's strange that this error is labeled as a provider error when it's fairly likely not a provider error. I think the most likely cause will be users thinking there is a variable name foo and there not actually being one.

It's not a huge deal that this gets rendered as provider("no variable named 'foo'") - the error is clear enough - it's just a bit strange is all.

I personally don't really see potential confusion from having another error case, but I guess I'm fine letting this go if everyone disagrees with me.

itowlson

A couple of nits but otherwise LGTM!

crates/loader/Cargo.toml

crates/trigger/src/runtime_config.rs

Signed-off-by: Brian H <[email protected]>

fibonacci1729 force-pushed the variables branch from a74e546 to 8616385 Compare October 19, 2023 17:29

fibonacci1729 requested review from rylev, itowlson and lann October 19, 2023 17:29

fibonacci1729 commented Oct 19, 2023

View reviewed changes

crates/trigger/src/runtime_config/variable_provider.rs Outdated Show resolved Hide resolved

fibonacci1729 force-pushed the variables branch from 8616385 to de0c39b Compare October 19, 2023 17:44

itowlson reviewed Oct 19, 2023

View reviewed changes

fibonacci1729 force-pushed the variables branch from de0c39b to cbf2af1 Compare October 19, 2023 19:05

lann reviewed Oct 19, 2023

View reviewed changes

crates/trigger/src/runtime_config.rs Outdated Show resolved Hide resolved

fibonacci1729 force-pushed the variables branch 3 times, most recently from 3881bb0 to fc5cb48 Compare October 19, 2023 20:40

rylev requested changes Oct 20, 2023

View reviewed changes

fibonacci1729 force-pushed the variables branch 2 times, most recently from 469bf4f to dd5125d Compare October 20, 2023 16:43

fibonacci1729 requested a review from rylev October 20, 2023 17:06

fibonacci1729 force-pushed the variables branch from dd5125d to 93472b0 Compare October 20, 2023 19:48

fibonacci1729 requested review from itowlson and lann October 20, 2023 19:48

rylev approved these changes Oct 23, 2023

View reviewed changes

fibonacci1729 force-pushed the variables branch from 93472b0 to 462c2df Compare October 23, 2023 15:08

lann approved these changes Oct 23, 2023

View reviewed changes

fibonacci1729 force-pushed the variables branch from 462c2df to 4f2a3b3 Compare October 23, 2023 17:15

itowlson approved these changes Oct 23, 2023

View reviewed changes

crates/loader/Cargo.toml Outdated Show resolved Hide resolved

crates/trigger/src/runtime_config.rs Outdated Show resolved Hide resolved

crates/trigger/src/runtime_config.rs Outdated Show resolved Hide resolved

fibonacci1729 force-pushed the variables branch 2 times, most recently from 2427936 to f05a6ea Compare October 23, 2023 21:34

Rename config to variables

7c6d865

Signed-off-by: Brian H <[email protected]>

fibonacci1729 force-pushed the variables branch from f05a6ea to 7c6d865 Compare October 23, 2023 22:30

fibonacci1729 merged commit 42e8ef4 into fermyon:main Oct 24, 2023
9 checks passed

fibonacci1729 deleted the variables branch October 24, 2023 16:03

rylev mentioned this pull request Oct 24, 2023

Rename spin_config to spin_variables #1912

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rename config to variables #1918

Rename config to variables #1918

fibonacci1729 commented Oct 19, 2023 •

edited

Loading

fibonacci1729 commented Oct 19, 2023 •

edited

Loading

rylev left a comment

rylev Oct 20, 2023

lann Oct 20, 2023 •

edited

Loading

rylev Oct 23, 2023

fibonacci1729 Oct 23, 2023

rylev Oct 23, 2023

itowlson left a comment

Rename config to variables #1918

Rename config to variables #1918

Conversation

fibonacci1729 commented Oct 19, 2023 • edited Loading

fibonacci1729 commented Oct 19, 2023 • edited Loading

rylev left a comment

Choose a reason for hiding this comment

rylev Oct 20, 2023

Choose a reason for hiding this comment

lann Oct 20, 2023 • edited Loading

Choose a reason for hiding this comment

rylev Oct 23, 2023

Choose a reason for hiding this comment

fibonacci1729 Oct 23, 2023

Choose a reason for hiding this comment

rylev Oct 23, 2023

Choose a reason for hiding this comment

itowlson left a comment

Choose a reason for hiding this comment

fibonacci1729 commented Oct 19, 2023 •

edited

Loading

fibonacci1729 commented Oct 19, 2023 •

edited

Loading

lann Oct 20, 2023 •

edited

Loading