vaijosh commented on PR #3081: URL: https://github.com/apache/hugegraph/pull/3081#issuecomment-5054574230
> ### ๐จ Review summary > Important > > The cloud recovery path has blocking consistency, configuration, deletion-safety, release-metadata, and verification gaps. > > #### ๐ Risk dashboard > Signal Result > ๐ฆ Review gate **Blocked** > ๐ Actionable findings **9** (8 High, 1 Medium) > ๐งช Verification coverage **5** checks > โ ๏ธ Confirmed / limited checks **2 / 1** > #### ๐ฌ Coverage details > โ ๏ธ **S3 failure semantics** โ Confirmed > **Trace:** S3CloudStorageProvider upload, download, delete, and exception-classification paths, CloudStorageEventListener purge and hydration flows, AWS SDK 2.33.8 retry-code and file-transformer behavior > > **Conclusion:** Confirmed ignored per-object deletion errors, retryable error-code misclassification, and non-atomic download recovery risks; lower-impact lifecycle leads were dropped. > > โ ๏ธ **Release and E2E integrity** โ Confirmed > **Trace:** install-dist release LICENSE and known-dependencies inventory, cloud-storage Docker artifact build and recovery workflow, Maven provider packaging and test activation > > **Conclusion:** Confirmed the release dependency inventory mismatch and E2E false-green paths caused by suppressed build and wipe failures. > > ๐ก **Focused unit suites** โ Limited > **Trace:** hg-store-common cloud configuration, provider, and factory tests, hg-store-cloud-s3 exception-classification tests, hg-store-node listener, retry, metrics, tracker, configuration, and callback tests, hg-store-core BusinessHandlerImplTest > > **Conclusion:** Surefire reports recorded 184 tests with 0 failures, 0 errors, and 0 skipped. The suites do not cover concurrent metadata publication, comma-separated data roots, or partial S3 batch deletion. > > โ **Maven reactor** โ Clear > **Trace:** Full 44-project reactor with the cloud-s3 profile, hugegraph-store module ordering and dependency graph > > **Conclusion:** Maven validate completed successfully for all 44 reactor projects; the suspected duplicate cloud-s3 module activation did not reproduce. > > โ **Shell syntax** โ Clear > **Trace:** pd-entrypoint.sh, store-entrypoint.sh, test-graph-queries-and-sst.sh > > **Conclusion:** bash -n returned successfully for all three changed shell entrypoints and workflows. > > Warning > > **Verification limits** > > * No live Docker and MinIO recovery drill was run, so cluster recovery and purge behavior were not exercised end to end. > * The test environment used Java 17; the legacy JaCoCo agent emitted unsupported class-version instrumentation errors, so coverage instrumentation was not verified even though test assertions passed. > * S3SingleLargeFileE2ETest was intentionally not run because its default path generates a 20 GiB file. > * The independent recovery-consistency run timed out after 900 seconds and produced no usable result. > > ๐ค Codex review ยท **GPT-5.6 Sol** ยท effort: **xhigh** Hi @VGalaxies. Thanks for reviewing the changes. I have addressed all the review comments. Can you please review it again? -- 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] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
