Add toggle for bridge queue config #775
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…e crashes for default config Signed-off-by: Rhea Danzey <rdanzey@element.io>
Signed-off-by: Rhea Danzey <rdanzey@element.io>
Half-Shot
requested changes
Jun 8, 2023
| @@ -526,7 +529,7 @@ export class BridgeConfig { | |||
| @hideKey() | |||
| private readonly bridgePermissions: BridgePermissions; | |||
|
|
|||
| constructor(configData: BridgeConfigRoot, env?: {[key: string]: string|undefined}) { | |||
| constructor(configData: BridgeConfigRoot, env?: { [key: string]: string | undefined }) { | |||
| this.bridge = configData.bridge; | |||
| assert.ok(this.bridge); | |||
| this.github = configData.github && new BridgeConfigGitHub(configData.github); | |||
There was a problem hiding this comment.
Would suggest modifying line 552 so that if enabled is false, we treat it as { monolithic: true}
There was a problem hiding this comment.
Handled in latest commit - I'm not a great JS dev, feel free to critique
… if queue is disabled Signed-off-by: Rhea Danzey <rdanzey@element.io>
Is there a special invocation I need to make it match up with your conventions or are they not in the repo? I think I just have blanket auto formatting on everything in VSCode somehow... |
Signed-off-by: Rhea Danzey <rdanzey@element.io>
Signed-off-by: Rhea Danzey <rdanzey@element.io>
Signed-off-by: Rhea Danzey <rdanzey@element.io>
Signed-off-by: Rhea Danzey <rdanzey@element.io>
Signed-off-by: Rhea Danzey <rdanzey@element.io>
…ns without redis Signed-off-by: Rhea Danzey <rdanzey@element.io>
Signed-off-by: Rhea Danzey <rdanzey@element.io>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
With the 2.6.0 release it looks like worker support with Redis was configured. With the Helm Chart using the default configuration, the values.yaml has a configured
queueblock since it's in theconfig.sample.ymlfile.Given that the chart doesn't yet deploy Redis, this results in a broken config by default unless
.Values.hookshot.config.queueis set to{}as user-specified values will be merged on top of chart default values.This will also break user configurations if they set up shop using the
config.sample.ymlfile as it exists prior to this commit.We're just adding an
enabledflag to thequeueconfig key and defaulting it to false - That way Hookshot should never expect a Redis service to be available unless explicitly configured and enabled.