bitflicker64 commented on code in PR #787:
URL:
https://github.com/apache/hugegraph-toolchain/pull/787#discussion_r4207004753
##########
.github/workflows/ci.yml:
##########
@@ -79,15 +79,15 @@ jobs:
with:
repository: apache/hugegraph
ref: refs/tags/1.7.0
- expected-commit: b12425c2032bf0d21a97b8221f42a18055c2982f
+ expected-commit: b12425
- name: Resolve locked Java17 candidate baseline
id: hubble-baseline
if: steps.scope.outputs.needsFixture == 'true'
uses: ./.github/actions/get-hugegraph-commit
with:
repository: apache/hugegraph
- ref: d9abcd4317fb36128e7e4d209139ec7f3a28cdfc
- expected-commit: d9abcd4317fb36128e7e4d209139ec7f3a28cdfc
+ ref: d9abcd
Review Comment:
Important: A six-character lock breaks as soon as the prefix stops being
unique, and anyone can make that happen. GitHub answers an ambiguous prefix
with 422: `gh api repos/apache/hugegraph/commits/d9abc` returns `No commit
found for SHA: d9abc` while `d9abcd` resolves, so another `d9abc...` commit
already exists. Commits from fork PRs resolve in the same namespace (`gh api
repos/apache/hugegraph/commits/3900381` returns the head of closed fork PR
apache/hugegraph#3274). A 24-bit prefix takes seconds to brute-force with a
commit-message nonce, so one PR to apache/hugegraph with a `d9abcd...` (or
`b12425...`) commit makes this plan step, every `setup-candidate-sdk` caller
(default `d9abcd`), `checkout-server.sh` and both Dockerfiles fail until this
repo is edited. Also, since `ref` and `expected-commit` both resolve the same
prefix through the same API, the expected-commit check no longer adds an
independent lock. Please keep the full 40-character SHAs in
`ref`/`expected-commit` he
re (and for `b12425` on line 82), in the `setup-candidate-sdk` defaults and in
the Dockerfile arguments, and shorten only the logged identity.
##########
.github/scripts/verify_candidate_image_sdk.py:
##########
@@ -20,10 +20,14 @@
import argparse
import hashlib
import json
+import os
+import re
+from functools import lru_cache
+import urllib.request
from pathlib import Path
REPOSITORY = "apache/hugegraph"
-COMMIT = "d9abcd4317fb36128e7e4d209139ec7f3a28cdfc"
+COMMIT = "d9abcd"
Review Comment:
Important: With `COMMIT` shortened, `source_commit()` (line 75) calls
`https://api.github.com/repos/apache/hugegraph/commits/d9abcd` whenever
`CANDIDATE_SOURCE_COMMIT` is unset. Only `setup-candidate-sdk` exports that
variable, so both Dockerfiles' `RUN ... verify_candidate_image_sdk.py` steps
and the antrun `exec` calls in hugegraph-dist/pom.xml (three),
hugegraph-loader, hugegraph-tools and hubble-dist poms now need network access
to GitHub during packaging. Each is a new Python process, so `lru_cache` does
not help, and none send a token inside `docker build`, so they share the 60
requests per hour unauthenticated limit. An offline, proxied or rate-limited
`docker build` or `mvn package` that passed before now fails with `Invalid
candidate SDK manifest (URLError)`. `checkout-server.sh` adds another
unauthenticated call for the short argument. Please restore the full
40-character `COMMIT` so verification stays local, and drop
`_resolve_source_commit` for the standalone path.
--
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]
