bitflicker64 commented on PR #3449: URL: https://github.com/apache/hugegraph/pull/3449#issuecomment-6613363847
## Review summary: R2 (DELTA), head `857dd12c` **Verdict: both R1 findings are addressed. One new Important item should be settled before merge.** This round covers `6524cc14..857dd12c` plus re-checks of the R1 items. The R1 review at `6524cc14` is [here](https://github.com/apache/hugegraph/pull/3449#issuecomment-6611306324) and is unchanged. With Hubble persistence enabled, a saved Text2GQL API key now survives restarts and Pod replacement. The wrapper restores `0600` after `fsGroup` widens the key's permissions. Releases upgrading from before this PR re-enter the key once, as now documented. With `persistence.enabled=false`, the key and H2 are lost together on Pod replacement. - **R1 `fsGroup` permissions: fixed** for the same-UID case. [Thread](https://github.com/apache/hugegraph/pull/3449#discussion_r4458407777) - **R1 upgrade docs: fixed.** [Thread](https://github.com/apache/hugegraph/pull/3449#discussion_r4458407791) - **[Important, new]** A key file the current UID cannot `chmod` now stops all of Hubble from starting, not just Text2GQL. One way this happens is enabling Vermeer on an existing release. [Thread](https://github.com/apache/hugegraph/pull/3449#discussion_r4459423422) - **[Minor]** The PR body still says each Pod replacement generates a key, and that values are unchanged. The correction in the conversation could also go into the body. **Score: 8.5 / 10** (provisional, 2 of 4 reviews in). | Reviewer | Raw | Corrections | Adjusted | |---|---|---|---| | Reviewer A (gpt-6.1-terra) | 7.5 | none | 7.5 | | Reviewer B (model not reported) | 9.5 | none | 9.5 | **Checks:** one reviewer ran the 6 new Hubble wrapper tests (all passed). I confirmed the new item against the wrapper and the Vermeer `runAsUser` override. Nothing was run on a live cluster. -- 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]
