👷(CLDSRV-867) Clean old GCP bucket#6098
Conversation
Hello darkisdude,My role is to assist you with the merge of this Available options
Available commands
Status report is not available. |
Request integration branchesWaiting for integration branch creation to be requested by the user. To request integration branches, please comment on this pull request with the following command: Alternatively, the |
❌ 1 Tests Failed:
View the full list of 1 ❄️ flaky test(s)
To view more test analytics, go to the Test Analytics Dashboard |
|
Review summary: objectGetAttributes.js:119 Mutating singleton error objects pollutes shared state across requests, leaking CORS headers. Same issue at line 135. Clone the error before setting .responseHeaders. | bucketGet.js:158 Stray trailing comma after buildAttributesXml() creates a comma expression. Replace with semicolon. | routeBackbeat.js:728 errorMessage.includes repair is too broad. Remove it, keep only E11000 and duplicate key checks. | objectGetLegalHold.js:106 module.exports wrapper duplicates inner callback bridge (lines 20-25), making inner bridge dead code. Pick one pattern. | bucketGet.js:362 and objectGetLegalHold.js:51 promisify called on every request. Hoist to module level. -- Review by Claude Code |
Incorrect fix versionThe
Considering where you are trying to merge, I ignored possible hotfix versions and I expected to find:
Please check the |
Request integration branchesWaiting for integration branch creation to be requested by the user. To request integration branches, please comment on this pull request with the following command: Alternatively, the |
|
LGTM |
7f96ee6 to
c884f1c
Compare
| const genUniqID = () => uuidv4().replace(/-/g, ''); | ||
| const genUniqID = () => { | ||
| const runId = process.env.GITHUB_RUN_ID; | ||
| const suffix = uuidv4().replace(/-/g, '').slice(0, 8); |
There was a problem hiding this comment.
When GITHUB_RUN_ID is set, genUniqID produces only 8 hex characters of randomness (uuidv4().slice(0, 8)). Multiple test files call genBucketName at module-load time with the same testName within the same CI run, so the only uniqueness comes from those 8 chars. While ~4 billion values makes collisions very unlikely in practice, the previous code used the full 32-char UUID. Consider using a longer slice (e.g. 12-16 chars) to keep the same safety margin while still including the run ID.
— Claude Code
|
LGTM — clean PR that adds a GCP bucket cleanup job and improves bucket naming for traceability. One minor note: |
Incorrect fix versionThe
Considering where you are trying to merge, I ignored possible hotfix versions and I expected to find:
Please check the |
c884f1c to
a1961e0
Compare
Request integration branchesWaiting for integration branch creation to be requested by the user. To request integration branches, please comment on this pull request with the following command: Alternatively, the |
|
LGTM |
|
LGTM |
|
/approve |
Integration data createdI have created the integration data for the additional destination branches.
The following branches will NOT be impacted:
You can set option The following options are set: approve |
Build failedThe build for commit did not succeed in branch w/9.4/feature/CLDSRV-867/clean-gcp-bucket The following options are set: approve |
|
I have successfully merged the changeset of this pull request
The following branches have NOT changed:
Please check the status of the associated issue CLDSRV-867. Goodbye darkisdude. The following options are set: approve |
Pull request template
Description
Motivation and context
Clean GCP bucket older than one week - stuck from the CI and will never be removed.
Add the GITHUB Action ID in the bucket name to be able to identify the root cause.