A first attempt at using deser-hjson. Fixes #1270 #1433

dessalines · 2021-02-17T00:55:43Z

Haven't tried upgrading deps yet, but it works!

crates/utils/src/settings.rs

docker/lemmy.hjson

src/main.rs

Nutomic · 2021-02-18T14:50:35Z

crates/utils/src/settings.rs

@@ -56,7 +97,16 @@ pub struct EmailConfig {
 #[derive(Debug, Deserialize, Clone)]
 pub struct CaptchaConfig {
  pub enabled: bool,
-  pub difficulty: String, // easy, medium, or hard
+  pub difficulty: String,


Can we turn this into an enum?

I doubt it, since json / hjson doesn't support enums.

The API code does this:

let captcha = match captcha_settings.difficulty.as_str() { "easy" => gen(Difficulty::Easy), "medium" => gen(Difficulty::Medium), "hard" => gen(Difficulty::Hard), _ => gen(Difficulty::Medium), };

I mean in Rust, so that it gets serialized to string automatically.

crates/utils/src/settings.rs

Nutomic · 2021-02-18T16:25:23Z

Does this change still allow editing the config via /admin?

- Removing unwrap_or_defaults, using impl functions. - Reorganized settings

dessalines · 2021-02-19T01:07:41Z

ansible/templates/config.hjson

@@ -29,8 +29,9 @@
    # https://join.lemmy.ml/docs/en/federation/administration.html#instance-allowlist-and-blocklist
    #
    # comma separated list of instances with which federation is allowed
-    # allowed_instances: ""
+    # Only one of these blocks should be uncommented
+    # allowed_instances: ["instance1.tld","instance2.tld"]


hjson needs quoted strings if its in a list.

Nutomic · 2021-02-23T17:13:49Z

This looks actually pretty good, because it needs way less boilerplate for the settings compared to my approach. The only problem is that it doesnt support env vars. If you can add that somehow, then I would prefer your implementation (feel free to copy things from my branch).

dessalines · 2021-02-24T01:10:31Z

I'll try that out tmrw

dessalines · 2021-02-25T00:58:58Z

crates/utils/src/settings/mod.rs

+    let mut custom_config = from_str::<Settings>(&Self::read_config_file()?)?;
+
+    // Merge with env vars
+    custom_config.merge(envy::prefixed("LEMMY_").from_env::<Settings>()?);


Okay @Nutomic , got env vars working. I took this from your PR, and it worked (see below)

So it works for all config values, including nested ones? Can we revert the changes to configuration in api_tests/ and docker/ then?

We could, but since I already wrote the configs as proper hjson files, I don't think there's a reason to.

dessalines · 2021-02-25T00:59:28Z

docker/federation/lemmy_alpha.hjson

@@ -0,0 +1,36 @@
+{
+  port: 8541


I removed hostname from this file to test env vars.

dessalines · 2021-02-25T01:00:01Z

api_tests/prepare-drone-federation-test.sh

-  LEMMY_SETUP__ADMIN_USERNAME=lemmy_alpha \
-  LEMMY_SETUP__SITE_NAME=lemmy-alpha \
-  target/lemmy_server >/dev/null 2>&1 &
+  LEMMY_HOSTNAME="lemmy-alpha:8541" \


added an env var for hostname here, removed it from the config file

Nutomic · 2021-02-26T16:29:41Z

I tried to test the env vars by doing:

git checkout main -- api_tests
cd api_tests
./run-federation-test.sh

But the tests are all failing. After adding dbg!(&settings.federation().allowed_instances) in the code, it logs settings.federation().allowed_instances = None. So parsing of nested env vars doesnt seem to work yet (I tried both LEMMY_FEDERATION_ALLOWED_INSTANCES and LEMMY_FEDERATION__ALLOWED_INSTANCES).

dessalines · 2021-02-26T18:48:17Z

Remember that while every top level field is optional, not every field in the blocks are, so you might need to add the other required fields for that block.

Nutomic · 2021-03-01T13:34:08Z

crates/utils/src/settings/mod.rs

+      iframely_url: Some("http://iframely".into()),
+    }
+  }
+}


I would move struct Settings into structs.rs, and impl Default into defaults.rs, just to have everything in one place.

Nutomic · 2021-03-01T13:38:37Z

Ah you are using envy, no wonder that nested vars arent working then. There is no easy solution it seems, so lets merge with env parsing as is, and open an issue to improve it later (linking to softprops/envy#55). We also need to emphasize this as a breaking change in the changelog.

Edit: Actually that issue I linked has a (somewhat verbose) solution. Doesnt have to be part of this PR, but we should be able to implement it before v0.10.0 and avoid the breaking change.

…n-pwd-change * origin/main: revert Compose file version from 3.3 to 2.2 Adding more mem limits bump memory limit of iframely Remove extra category_id s . Fixes LemmyNet#1429 Fixing wrong user_ and community icon and banner urls. Remove category from activitypub context Adding a password length check to other API actions. (LemmyNet#1474) Update test script Use URL type in most outstanding struct fields (LemmyNet#1468) Forbid usage of unwrap Upgrade Rust version Rewrite settings implementation. Fixes LemmyNet#1270 (LemmyNet#1433) Rename `lemmy_structs` to `lemmy_api_structs` # Conflicts: # crates/db_schema/src/source/user.rs

A first attempt at using deser-hjson. Fixes #1270

f19c7a2

dessalines requested a review from Nutomic February 17, 2021 00:55