Sender | Message | Time |
---|---|---|
31 Aug 2023 | ||
@ajs124:ajs124.de | 09:21:13 | |
@ajs124:ajs124.de | 10:25:17 | |
@ajs124:ajs124.de | 11:15:20 | |
@ajs124:ajs124.de | 11:44:06 | |
@ajs124:ajs124.de left the room. | 13:14:12 | |
@ajs124:ajs124.de joined the room. | 13:14:44 | |
@ajs124:ajs124.de | 13:37:50 | |
@ajs124:ajs124.de | 14:54:50 | |
@ajs124:ajs124.de | 15:18:07 | |
Moritz Hedtke removed their display name moritz.hedtke. | 16:13:27 | |
1 Sep 2023 | ||
@andreas.schraegle:helsinki-systems.de left the room. | 10:14:36 | |
10 Sep 2023 | ||
Dandellion | ma27: Could you elaborate what you meant by
| 22:01:18 |
Dandellion | * ma27: Could you elaborate on what you meant by
| 22:01:23 |
Dandellion | Is it this part? | 22:12:43 |
Dandellion | I will reiterate again that my alternative was there to show off a IMO more preferable API, and that I'm open to changing really most implementation details | 22:17:24 |
Dandellion | * I will reiterate again that my alternative was there to show off an IMO more preferable API, and that I'm open to changing really most implementation details | 22:17:32 |
11 Sep 2023 | ||
ma27 | In reply to @dandellion:dodsorf.as ok I wasn't too precise there. I meant the import of the worker-module which creates these units, sorry. The module-system consists of functions with certain arguments (defined by additionally, are there that many differences left now? the other PR also did fwiw I'm not sure if more opinionated things such as nginx config generation for load-balancing of workers should be in a nixos module (at least as long as we don't have rfc42 for nginx - otherwise I'm not too worried about such an idea) because it's far too easy then to run into a problem that isn't covered by the module and then the config will be very hard to customize from my experience with maintaining nextcloud which actually ships with a full-blown nginx config. I may be wrong though, so if you think you have a good design for that, feel free to file a patch, I'm happy to take a look :) | 13:54:16 |
ma27 | Also, I'll take a final look at the PR, but I think it's in a mergeable state now. I'll wait with merging until tonight (in CEST) though because I'd like to hear if you think that there are design flaws that shouldn't be merged. | 13:55:26 |
⛧-440729 [sophie] (it/its) joined the room. | 14:21:06 | |
Dandellion | Thanks for clarifying. I was not aware that this PR does attrsOf now, that's great! I'm not going to block this at all then The only difference left is probably that I use I'll do a more thourough review, but I generally support merging this more complete implementation | 15:54:18 |
Dandellion | I do think the module is growing to becoming kind of unwieldy so it'd be nice if we could figure out how to split it up like I attempted to with the import workers.nix stuff | 15:55:37 |
ma27 | I don't think it's that bad: I mean, if we'd talk about the sizes of *-packages.nix, then I'd agree (because most editors and LSPs are getting horribly slow then), but this doesn't seem to be the case here. Also, in languages with rather underdeveloped editor integrations I think splitting things off prematurely only makes it harder to find stuff IMHO. I think there's a reasonable boundary though: the current module is responsible for setting up synapse itself (including workers) which is tightly coupled (because e.g. the systemd units are quite similar). If we come up with a good design for setting up load balancing w/ nginx of HTTP locations to workers and/or the main process, then we'd have something that clearly belongs into its own file. Also, if the module becomes too complex eventually, we can also consider moving the options into its own file (right now we wouldn't gain much from it because the majority of the file consists of options).
It used to be a level deeper, but I suggested to remove it (there was an enable option as well, but instead I'm still interested in your additional patches (just declaring that one needs N workers doing X and M workers doing Y seems pretty cool. Not sure if we'll find a reasonable abstraction for the nginx part w/o rfc42 for nginx, but we'll see and I'm happy to discuss this in a github issue first if you're not sure - asynchronous discussion is preferred over chat for that). I think this is a very big step forward now (after supporting synapse for a long time, one can trivially set up workers on NixOS now without knowledge about the inner workings of the synapse module - knowing how to write Nix and (basic) knowledge about synapse configuration is sufficient; it's type-checked, validated & well-documented) and these kinds of ideas are clearly follow-up work, so I'll stick to my plan and merge. | 19:27:53 |
16 Sep 2023 | ||
hexa | TIL: https://github.com/matrix-org/synapse/blob/develop/flake.nix | 17:18:04 |
hexa | created by anoadragon453 | 17:18:38 |
Sumner Evans | Yeah. It's kinda a weird setup for dev tbh. But if you modify a couple things it's usable. | 17:31:45 |
18 Sep 2023 | ||
hexa | worker support 👏 | 17:54:49 |
hexa | thanks everyone | 17:55:09 |
hexa | now, who will manage that nasty nginx vhost for me? 🤡 | 17:55:35 |
ma27 | I'm sure there are at least three people with config for that :> | 17:59:07 |
Dandellion | I think Ma27 seemed quite negative about this generally, But personally I think just putting the maps inside But I'll be changing my module to match the nixpkgs implementation and work towards upstreaming the autoconfig stuff | 18:01:57 |