bitflicker64 commented on code in PR #3282:
URL: https://github.com/apache/hugegraph/pull/3282#discussion_r4201888027
##########
helm/hugegraph/Chart.yaml:
##########
@@ -19,8 +19,8 @@ apiVersion: v2
name: hugegraph
description: Helm chart for Apache HugeGraph HStore cluster (PD + Store +
Server)
type: application
-version: 0.1.0
-appVersion: "latest"
+version: 0.1.1
+appVersion: "1.8.0"
Review Comment:
Important: `appVersion: "1.8.0"` points at images that are not published
yet, and two parts of the chart consume it right away.
1. `hugegraph.image` in `_helpers.tpl:1001` falls back with `default
.appVersion $img.tag`. Before this PR, an empty `pd/store/server.image.tag`
(for example `--set server.image.tag=` or a values file that clears it)
rendered `:latest`. Now it renders `hugegraph/server:1.8.0`, and the new
`image_digest_test.yaml` cases assert exactly that. Docker Hub returns 404 for
`hugegraph/pd:1.8.0`, `hugegraph/store:1.8.0` and `hugegraph/server:1.8.0`
today, so such an install or upgrade ends in `ImagePullBackOff`.
2. `hugegraph.labels` (`_helpers.tpl:48`) puts `app.kubernetes.io/version:
"1.8.0"` on every object, while the default `tag: latest` pods run whatever
`latest` is. Tooling that reads that label (dashboards, `kubectl get -L`,
policy checks) reports a version the pods are not running.
The values comments already say the switch to inheriting `appVersion` waits
for the published images, so the metadata should wait too. Requested change:
keep `appVersion: "latest"` (chart `version` can still move to 0.1.1) and bump
`appVersion` in the same post-release change that empties the tags. Then drop
or retarget the `inherits appVersion ... 1.8.0` cases and the 1.8.0 label
assertions and snapshots. If you want to keep 1.8.0 now, add a guard that fails
rendering when a component tag is empty and `appVersion` has no published
image, so the fallback cannot silently produce an unpullable reference.
##########
README.md:
##########
@@ -157,6 +157,8 @@ flowchart TB
## Quick Start
+<!-- TODO: update release downloads, package names, image tags and version
examples after 1.8.0 is published. -->
Review Comment:
Minor: the post-release TODO markers cover this README and two Store guides,
but other 1.7.0 references that the release sweep has to change carry no marker:
- `hugegraph-pd/README.md:4` and `hugegraph-store/README.md:4`: the
`version-1.7.0` badges describe the module version, which is 1.8.0 on master
after this PR.
- `hugegraph-server/README.md:37,40,48`, `docker/README.md:205` and
`hugegraph-server/hugegraph-dist/docker/README.md` (lines 7, 15, 25, 38, 49,
66, 76, 128, 156): image tags and `HUGEGRAPH_VERSION=1.7.0` examples.
- `hugegraph-store/docs/operations-guide.md:630-765` and
`development-guide.md`: `apache-hugegraph-store-incubating-1.7.0` paths.
- Comments that name the build output directory:
`hugegraph-cluster-test/.../BaseSimpleTest.java:48`,
`BaseMultiClusterTest.java:41` (`apache-hugegraph-ct-1.7.0`) and
`hugegraph-dist/src/assembly/travis/test-start-hugegraph-pd.sh:25`.
Requested change: bump the two badges (and the build-directory comments)
now, since they describe the source tree rather than a published artifact. Add
the same TODO marker to the server and docker READMEs and the remaining Store
guides, so whoever does the post-release update finds every place with one grep.
##########
install-dist/release-docs/LICENSE:
##########
@@ -511,7 +511,6 @@ the corresponding per-component license file.
https://central.sonatype.com/artifact/org.apache.commons/commons-collections4/4.4
-> Apache 2.0
https://central.sonatype.com/artifact/org.apache.commons/commons-compress/1.21
-> Apache 2.0
https://central.sonatype.com/artifact/org.apache.commons/commons-configuration2/2.10.1
-> Apache 2.0
Review Comment:
Minor: this removes `commons-configuration2/2.8.0` (and
`javassist/3.25.0-GA` below) from the LICENSE list and from
`known-dependencies.txt`, but the per-artifact files
`install-dist/release-docs/licenses/LICENSE-commons-configuration2-2.8.0.txt`
and `install-dist/release-docs/licenses/LICENSE-javassist-3.25.0-GA.txt` are
still in the tree. `git grep -nE 'configuration2.?2\.8|javassist.?3\.25'
5854beb` now finds nothing that refers to them, so the release docs ship
license texts for jars that are no longer bundled.
Requested change: delete those two files in this PR so the license inventory
stays in step with the dependency list.
--
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]
