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

feat(workers): Optional Admin and Console Server Workers #24

Merged
merged 1 commit into from
Sep 3, 2024

Conversation

Antonio-RiveroMartnez
Copy link
Collaborator

In order to facilitate the work for #23 we are adding two more env variables to the config so we can opt in/out from the Admin and Console server workers due to their optional nature. By default you would be running all workers as currently does the app.

Ideally we could control which server workers to run when serving your self hosted solution via a Feature Flag, but currently the Feature Flags are managed by LaunchDarkly which would be an optional service after we address #2.

- Make Admin and Console workers optional via env variable
@johnbwoodruff
Copy link
Collaborator

@Antonio-RiveroMartnez have you been able to validate that running things works correctly? Just curious if there are any assumptions in the other required pieces that these services are running.

@Antonio-RiveroMartnez
Copy link
Collaborator Author

Antonio-RiveroMartnez commented Sep 3, 2024

@Antonio-RiveroMartnez have you been able to validate that running things works correctly? Just curious if there are any assumptions in the other required pieces that these services are running.

It works correctly for our use case that is only having the API up and served so it can be consumed by the SDK and CLI commands for both SingleMode and ClusterMode. Those other two server workers (Admin and Console) are not needed hence the env variable to opt out. This also allowed us to use less resources.

Here you can see how the initialization completes without Admin nor Console:
Screenshot 2024-09-03 at 6 21 29 PM

@eschutho eschutho merged commit f6ca50d into getcord:main Sep 3, 2024
@eschutho eschutho deleted the optional_workers branch September 3, 2024 17:05
jwatzman added a commit to jwatzman/cord that referenced this pull request Oct 12, 2024
The optional workers in getcord#24 doesn't quite work right: because env
variables are always strings, `generate-dotenv.cjs` will set these two
new env vars to the string `'false'` -- which is truthy. So the console
and admin workers will always be deactivated unless we set the value to
the string `''` or similar, which is confusing for these
boolean-sounding settings.

Instead, we can just check if the port number is set, and skip if not,
which is nice that we don't need a new env var -- you can just set the
port number to the empty string to deactivate.  This does have a similar
issue, in that if you set the port to `0'`, it will still try to
activate the optional worker -- but `parseListenPort` will immediately
yell that `0` is an invalid port so at least you know immediately.
jwatzman added a commit to jwatzman/cord that referenced this pull request Oct 12, 2024
The optional workers in getcord#24 doesn't quite work right: because env
variables are always strings, `generate-dotenv.cjs` will set these two
new env vars to the string `'false'` -- which is truthy. So the console
and admin workers will always be deactivated unless we set the value to
the string `''` or similar, which is confusing for these
boolean-sounding settings.

Instead, we can just check if the port number is set, and skip if not,
which is nice that we don't need a new env var -- you can just set the
port number to the empty string to deactivate.  This does have a similar
issue, in that if you set the port to `0'`, it will still try to
activate the optional worker -- but `parseListenPort` will immediately
yell that `0` is an invalid port so at least you know immediately.
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.

3 participants