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.

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

Allow running the webserver portion standalone. #184

Merged
merged 13 commits into from
Nov 28, 2023
Merged

Conversation

Half-Shot
Copy link
Contributor

This is currently a draft as I work on the interdependencies between the backend parts and the frontend, and figuring out how to handle both.

I had quite a few challenges actually getting this going on my laptop, so I made a few bold decisions which are documented in my commits. Nothing hair raising, but quite a few major bumps as our dependencies had started to rot. Ideally this would land in a different PR, but I'd rather not overcomplicate things at present.

The new system works by specifying a runmode in your config or env vars, which allows the service to either run "normally" or in a "webserver" configuration. This means that when we run this in a kube environment, we just supply a different env.

@Half-Shot
Copy link
Contributor Author

I'm going to stick this behind #186 as it's likely going to be impacted by it.

@Half-Shot Half-Shot marked this pull request as ready for review November 10, 2023 15:16
@Half-Shot Half-Shot requested a review from a team as a code owner November 10, 2023 15:16
Copy link
Contributor

@reivilibre reivilibre left a comment

Choose a reason for hiding this comment

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

admittedly there's a lot going on here and I don't quite follow it all, but nothing stands out as harmful so let's go with it and we can tweak later if needed

@@ -299,3 +299,6 @@ ircBridge: null

# The allowed channel prefix of the bot
#channelPrefix: '#conference-'

templatesPath: ./srv
runMode: normal
Copy link
Contributor

Choose a reason for hiding this comment

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

I would have been tempted to make this a CLI argument instead so you don't need two config files, but is that reasonable?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

At the moment it's both an env file and a config argument, the idea being we have one config for both processes and just switch on a env...however I think the config field is probably pointless. Let's just have an env flag for easy kubernetes-ing.

Comment on lines +181 to +184
// Use first returned IP.
hostname: ip[0],
Copy link
Contributor

Choose a reason for hiding this comment

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

this doesn't seem to match the commit comment, also I don't see how this makes sense at current as it seems like it'd just choose the first character... (maybe it will be obvious with later commits).

@Half-Shot Half-Shot merged commit d20c012 into main Nov 28, 2023
4 checks passed
@Half-Shot Half-Shot deleted the hs/split-out-webserver branch November 28, 2023 22: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