-
Notifications
You must be signed in to change notification settings - Fork 19
feat: make mqs infra provisioning configurable #560
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
base: healthcheck
Are you sure you want to change the base?
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
fmvilas
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggesting an alternative name. Still looking at the code though.
| | `LOG_MAX_CONCURRENCY` | Maximum number of log writing operations to process concurrently. | `1` | No | | ||
| | `MAX_DESTINATIONS_PER_TENANT` | Maximum number of destinations allowed per tenant/organization. | `20` | No | | ||
| | `MAX_RETRY_LIMIT` | Maximum number of retry attempts for a single event delivery before giving up. Ignored if retry_schedule is provided. | `10` | No | | ||
| | `MQS_SHOULD_MANAGE` | Whether Outpost should create and manage message queue infrastructure. Set to false if you manage infrastructure externally (e.g., via Terraform). Defaults to true for backward compatibility. | `nil` | No | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about this?
| | `MQS_SHOULD_MANAGE` | Whether Outpost should create and manage message queue infrastructure. Set to false if you manage infrastructure externally (e.g., via Terraform). Defaults to true for backward compatibility. | `nil` | No | | |
| | `MQS_AUTO_CREATE` | Whether Outpost should create and manage message queue infrastructure. Set to false if you manage infrastructure externally (e.g., via Terraform). Defaults to true for backward compatibility. | `nil` | No | |
Or this?
| | `MQS_SHOULD_MANAGE` | Whether Outpost should create and manage message queue infrastructure. Set to false if you manage infrastructure externally (e.g., via Terraform). Defaults to true for backward compatibility. | `nil` | No | | |
| | `MQS_AUTO_MANAGE` | Whether Outpost should create and manage message queue infrastructure. Set to false if you manage infrastructure externally (e.g., via Terraform). Defaults to true for backward compatibility. | `nil` | No | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or even this?
| | `MQS_SHOULD_MANAGE` | Whether Outpost should create and manage message queue infrastructure. Set to false if you manage infrastructure externally (e.g., via Terraform). Defaults to true for backward compatibility. | `nil` | No | | |
| | `MQS_AUTO_PROVISION` | Whether Outpost should create and manage message queue infrastructure. Set to false if you manage infrastructure externally (e.g., via Terraform). Defaults to true for backward compatibility. | `nil` | No | |
fmvilas
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Love this refactor, @alexluong 👏
It makes things way more readable now. You can't imagine how much I appreciate this being new with this codebase 😄 🙏
| if err := a.PreRun(ctx); err != nil { | ||
| return err | ||
| } | ||
| defer a.PostRun(ctx) | ||
|
|
||
| return a.run(ctx) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Love this new lifecycle separation. Looks much cleaner.
resolves #547
introduces
MQS_SHOULD_MANAGEenvironment variable (open to changing to a better naming convention) which allows users to self-manage the mqs infra themselves