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

Filesystem config improvements #604

Merged
merged 9 commits into from
Oct 9, 2024
Merged

Filesystem config improvements #604

merged 9 commits into from
Oct 9, 2024

Conversation

Gnuxie
Copy link
Member

@Gnuxie Gnuxie commented Oct 8, 2024

Part of: the-draupnir-project/planning#30

Startup:

Tue, 08 Oct 2024 11:28:13 GMT [INFO] [Draupnir config] Configuration meta: {
  "configPath": "/home/user/experiments/Draupnir/config/harness.yaml",
  "isDraupnirConfigOptionUsed": false,
  "isAccessTokenPathOptionUsed": false,
  "isPasswordPathOptionUsed": false
}
Tue, 08 Oct 2024 11:28:13 GMT [WARN] [Draupnir config] DEPRECATED Starting Draupnir without the --draupnir-config option is deprecated. Please provide Draupnir's configuration explicitly with --draupnir-config. config path used: /home/user/experiments/Draupnir/config/harness.yaml

Crash:

Tue, 08 Oct 2024 11:39:38 GMT [INFO] [Draupnir config] non-default configuration properties: {
  "homeserverUrl": "http://localhost:8081",
  "rawHomeserverUrl": "http://localhost:8081",
  "pantalaimon": {
    "use": true,
    "username": "mjolnir",
    "password": "REDACTED"
  },
  "dataPath": "./test/harness/mjolnir-data/",
  "acceptInvitesFromSpace": "!example:example.org",
  "managementRoom": "#moderators:localhost:9999",
  "logLevel": "DEBUG",
  "commands": {
    "additionalPrefixes": [
      "mjolnir_bot"
    ]
  },
  "protections": {
    "wordlist": {
      "words": [
        "CaSe",
        "InSeNsAtIve",
        "WoRd",
        "LiSt"
      ]
    }
  },
  "admin": {
    "enableMakeRoomAdminCommand": true
  },
  "web": {
    "enabled": true,
    "port": 8082,
    "address": "0.0.0.0",
    "abuseReporting": {
      "enabled": true
    }
  },
  "configMeta": {
    "configPath": "/home/user/experiments/Draupnir/config/harness.yaml",
    "isDraupnirConfigOptionUsed": false,
    "isAccessTokenPathOptionUsed": false,
    "isPasswordPathOptionUsed": false
  }
}
Tue, 08 Oct 2024 11:39:38 GMT [INFO] [Draupnir config] Configuration meta: {
  "configPath": "/home/user/experiments/Draupnir/config/harness.yaml",
  "isDraupnirConfigOptionUsed": false,
  "isAccessTokenPathOptionUsed": false,
  "isPasswordPathOptionUsed": false
}
/home/user/experiments/Draupnir/src/config.ts:342
  throw new TypeError("Something broke")
        ^
TypeError: Something broke
    at readConfigSource (/home/user/experiments/Draupnir/src/config.ts:342:9)
    at configRead (/home/user/experiments/Draupnir/src/config.ts:347:18)
    at /home/user/experiments/Draupnir/test/integration/manualLaunchScript.ts:22:28
    at Object.<anonymous> (/home/user/experiments/Draupnir/test/integration/manualLaunchScript.ts:32:3)
    at Module._compile (node:internal/modules/cjs/loader:1369:14)
    at Module.m._compile (/home/user/experiments/Draupnir/node_modules/ts-node/src/index.ts:1618:23)
    at Module._extensions..js (node:internal/modules/cjs/loader:1427:10)
    at Object.require.extensions.<computed> [as .ts] (/home/user/experiments/Draupnir/node_modules/ts-node/src/index.ts:1621:12)
    at Module.load (node:internal/modules/cjs/loader:1206:32)
    at Function.Module._load (node:internal/modules/cjs/loader:1022:12)
Waiting for the debugger to disconnect...
error Command failed with exit code 1.

Gnuxie added 9 commits October 7, 2024 19:47
@Gnuxie
Rename read to configRead as it should have always been.
c0495d6
@Gnuxie
Got a way to extract non-default values.
10e89cc 
Now let's try unknown configuration values.
@Gnuxie
Show unknown property paths with a warning.
83c6006 
Now we just need to make this scrap available in commands.
@Gnuxie
Remove the old Mjolnir horrible RUNTIME client.
f6c5084
@Gnuxie
Make the path that is used to load the config available.
73a2368
@Gnuxie
Warn when --draupnir-config isn't used.
ed01664
@Gnuxie
Introduce configMeta so that we can log meta on process.exit later.
eae8588
@Gnuxie
Only show non-default config values when draupnir is exiting.
dc7f59e 
to reduce noise.
@Gnuxie
Get consistent with logging.
30bb95e 
So it turns out that mps4bot-sdk is using a different instance
of the bot-sdk module than Draupnir, i think.

Since we used to tell MPS's logger to use the bot-sdk's `LogService`,
but the `setLogger` that was used was obviously inconsistent with
Draupnir's.

Obviously the bot-sdk should be a peer dependency in the bot-sdk
to prevent this happening in future.
@Gnuxie Gnuxie requested review from FSG-Cat and MTRNord October 8, 2024 11:40
Gnuxie
Gnuxie commented Oct 8, 2024
View reviewed changes
src/config.ts
Comment on lines +264 to +273
function logNonDefaultConfiguration(config: IConfig): void {
log.info(
"non-default configuration properties:",
JSON.stringify(getNonDefaultConfigProperties(config), null, 2)
);
}

function logConfigMeta(config: IConfig): void {
log.info("Configuration meta:", JSON.stringify(config.configMeta, null, 2));
}
Copy link
Member Author

Choose a reason for hiding this comment

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

@MTRNord @FSG-Cat your review has been requested because I just wanted you to see and give thoughts on the log output shown in the PR description

Copy link
Member

Choose a reason for hiding this comment

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

Meow when is this shown?

Regular exit via Systemd shutting the bot down or only times when the bot shuts down unexpectedly.

Copy link
Member Author

Choose a reason for hiding this comment

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

We don't intercept either SIGINT or SIGTERM cos that sounded kinda sketchy. It's only when draupnir crashes does the stuff get logged.

Copy link
Member

Choose a reason for hiding this comment

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

This behavior of only showing on crash makes sense to me.

@Gnuxie Gnuxie merged commit 4015543 into main Oct 9, 2024
6 checks passed
@Gnuxie Gnuxie deleted the gnuxie/filesystem-config-improvements branch October 9, 2024 10:38
This was referenced Oct 9, 2024
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@FSG-Cat FSG-Cat Awaiting requested review from FSG-Cat

@MTRNord MTRNord Awaiting requested review from MTRNord

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
@Gnuxie @FSG-Cat