genegr commented on PR #13049: URL: https://github.com/apache/cloudstack/pull/13049#issuecomment-4302500106
Pushed a new revision rebased onto `apache/cloudstack:4.20` per @winterhazel's request, and addressed the new round of Copilot comments on `delete()`: - **`getMessage()` instead of `e.toString()`** in both delete-time benign-error catches (snapshot and volume), so the substring check is no longer coupled to JVM exception formatting. - **UTC deletion-timestamp suffix** via `DateTimeFormatter.withZone(ZoneOffset.UTC)`, so the rename is stable across management servers in different timezones (and across DST changes). Verified end-to-end: a delete issued at 01:11:34 BST landed on the array as `cloudstack::vol-…-20260423001134` — UTC, as expected. - **Sharper inline comment** on the snapshot branch: explicitly says FlashArray volume/snapshot names must match `[A-Za-z0-9_-]` and start/end with an alphanumeric, so the rename target would have an embedded `.` which the array rejects. Also dropped the now-unused `java.text.SimpleDateFormat` import (caught by checkstyle locally). ## Skipped: URL-encoding the `names=` query param (Copilot lines 214/242/247) Tested empirically against Purity 6.7.7: PATCH with `names=cloudstack::vol-…` (unencoded) and PATCH with `names=cloudstack%3A%3Avol-…` (URL-encoded) hit the **same target** and returned identical responses. `:` is RFC 3986-legal in the query component, and Purity decodes correctly either way. The current unencoded form is functionally fine. I considered applying the encoding for defense-in-depth but it would have to be applied **consistently across every `names=` callsite in the file**, not just the three Copilot flagged. That's a wider refactor than this fix-PR is scoped for. Happy to do it as a follow-up PR if you'd prefer — but it'd land in its own commit/PR rather than be folded in here. ## Skipped: structured exception type (Copilot line 221) Same reasoning — moving from substring-match to a typed exception with HTTP status would be a refactor of every `PATCH/POST/GET/DELETE` helper in the adapter, well beyond this PR. The smaller `getMessage()` swap above gives ~80% of the safety with zero refactor cost. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
