♻️ refactor new datacenter workflow #3964
Conversation
Bundles Sizes Evolution
🚀 CPU PerformancePending... 🧠 Memory PerformancePending... |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
.gitlab/deploy-auto.yml
Outdated
| - yarn | ||
| - yarn build:bundle | ||
| - node ./scripts/deploy/deploy-prod-dc.ts v${VERSION%%.*} $UPLOAD_PATH true | ||
| - node ./scripts/deploy/deploy-prod-dc.ts v${VERSION%%.*} $UPLOAD_PATH --withMonitorChecks true |
There was a problem hiding this comment.
The deploy job now invokes deploy-prod-dc.ts with --withMonitorChecks true, but the script switched to parseArgs({ type: 'boolean' }). For boolean options, parseArgs does not accept a space‑separated value, so this call is parsed as a positional "true" and Node throws ERR_PARSE_ARGS_UNEXPECTED_POSITIONAL before any deployment commands run. As written the automatic deploy pipeline will fail immediately. The command needs to use --withMonitorChecks/--no-withMonitorChecks or --withMonitorChecks=true instead (same problem exists in the manual pipeline).
Useful? React with 👍 / 👎.
| } = parseArgs({ | ||
| allowPositionals: true, | ||
| options: { | ||
| withMonitorChecks: { | ||
| type: 'boolean', | ||
| default: true, | ||
| }, | ||
| }, | ||
| }) |
There was a problem hiding this comment.
issue: The AI is kind of right here, what you are trying to do won't work.
The simplest solution is to remove the default: true here, and call deploy-prod-dc script with --withMonitorChecks only when you want to check monitors (in the auto pipeline):
node ./scripts/deploy/deploy-prod-dc.ts v${VERSION%%.*} $UPLOAD_PATH --withMonitorCheck
If you want to keep the default: true, you could enable allowNegative in parseArgs and invoke the script with --no-withMonitorCheck in the manual pipeline:
node ./scripts/deploy/deploy-prod-dc.ts v${VERSION%%.*} $UPLOAD_PATH --no-withMonitorCheck
Also, nitpicking a bit, but it could be nicer to format the option name as --monitor-check or something.
| * - Telemetry errors on specific org | ||
| * - Telemetry errors on specific message | ||
| */ | ||
| export const monitorIdsByDatacenter: Record<string, [number, number, number]> = { |
There was a problem hiding this comment.
nitpick: nitpicking and for the record: I think it would be nice to have a datacenterConfiguration object like:
export const datacenterConfigurations = {
us1: { site: 'xxx', monitorIds: [...] },
...
}There was a problem hiding this comment.
yeah, I think we actually need to discover these monitors in the middle/long term
There was a problem hiding this comment.
nitpick: this file contains more than sites now
… 'withMonitorChecks' for improved clarity and consistency in monitor checks during production deployment.
thomas-lebeau
changed the title
Motivation
As part of PR0, we're introduction a new DC
pr00test.staging.dog.This PR simplify a bit the runbook and setup the new DC.
It is recommended to review commit by commit.
Changes
monitorIdsByDatacenterwith other site configTest instructions
Checklist