feat(Storage): update StorageObject::exists to use HEAD request instead of GET#9196
feat(Storage): update StorageObject::exists to use HEAD request instead of GET#9196salilg-eng wants to merge 4 commits into
Conversation
…ad of GET Optimizes exists() check on GCS objects using lighter HEAD requests and resolves GCS retry conformance test failures.
Adds : array return type hint to ConnectionInterface and Rest connection client implementations, and fixes GCS retry conformance test TypeError crash by correctly returning GCS response headers array.
8627de5 to
488e1da
Compare
|
@salilg-eng Thanks so much for updating the PR! One more thing, there is one test that is failing that should not be failing (PHPSTAN static analysis), can you please update your branch with master? Either a rebase or a merge should be fine! |
|
Hello @Hectorhammett The single PHPStan Static Analysis failure is a preexisting upstream repository issue and is completely unrelated to the changes in this PR. This PR strictly modifies 4 files under the Storage package, which is 100% static-analysis green. The failure occurs entirely under the Spanner component due to the release of PHPStan 2.2.0 yesterday (May 28). Since the CI workflow uses composer update -d dev on every run, the build automatically pulled the new 2.2.0 release which has stricter sealed array validation rules, flagging preexisting array shapes inside Spanner/src/ValueMapper.php and Spanner/src/Operation.php. Upstream main commits analyzed before yesterday passed solely because they ran under the older 2.1.56 release. Ready for a final look and merge! |
|
Hi team, I did some Git and GitHub API forensics on the two failing Spanner files to understand exactly how they were merged in the first place without these errors blocking them, and why they are blocking my PR now. It turns out they represent two different situations:
💡 Gaps in Coverage & Prevention Recommendations:To prevent these pre-existing failures in other packages from constantly blocking active pull requests, the repo owners could consider two actions:
|
|
@Hectorhammett and @kalragauri The PHPStan check has now passed. This was verified following the merge of PR #9223 . 🎉 |
Fixes / Addresses
Addresses b/437200793
Behavior Change & Motivation
Previously,
StorageObject::exists()invokedgetObject()(making an HTTPGETrequest) to verify if an object existed. Following a server-side enhancement to Google Cloud Storage Audit Logs, failedGETrequests against non-existent objects started emitting severityERRORlog messages rather thanINFO.Because checking whether an object exists is intended for workflows where an object may not be present, it should not generate false-alarm error severity records in Cloud Audit Logs or trigger unnecessary payload data downloads.
Technical Implementation
headObject()definition inConnectionInterfaceand implemented it inRest.StorageObject::exists()to useheadObject()(HTTPHEAD) instead ofgetObject()(HTTPGET).